From f3637131f00a1457d4618f72f625d3ce1b9d0ad4 Mon Sep 17 00:00:00 2001 From: Timo K Date: Tue, 10 Dec 2024 17:59:11 +0100 Subject: [PATCH 1/8] Spsh 1580 (#831) Spsh 1580 --- .../.snapshot-dbildungs-iam-server.json | 57 ++++++++++----- migrations/Migration20241207094524-S.ts | 8 +++ seeding/dev/01/04_rolle.json | 10 +++ seeding/dev/01/05_technical-user.json | 10 ++- seeding/dev/01/06_personenkontext.json | 5 ++ seeding/prod/01/04_rolle.json | 10 +++ seeding/prod/01/05_technical-user.json | 9 ++- seeding/prod/01/06_personenkontext.json | 6 +- .../dbseed/domain/db-seed.service.spec.ts | 27 ++++++- src/console/dbseed/domain/db-seed.service.ts | 27 ++++--- .../domain/person-permissions.spec.ts | 70 +++++++++++++++++++ .../domain/person-permissions.ts | 22 ++++-- src/modules/person/api/person.controller.ts | 3 +- .../person.repository.integration-spec.ts | 17 +---- .../person/persistence/person.repository.ts | 3 +- .../api/personenkontext.controller.spec.ts | 6 +- .../api/personenkontext.controller.ts | 3 +- src/modules/rolle/domain/rolle.enums.ts | 1 + 18 files changed, 231 insertions(+), 63 deletions(-) create mode 100644 migrations/Migration20241207094524-S.ts diff --git a/migrations/.snapshot-dbildungs-iam-server.json b/migrations/.snapshot-dbildungs-iam-server.json index c9dc0d04c..2cb53599a 100644 --- a/migrations/.snapshot-dbildungs-iam-server.json +++ b/migrations/.snapshot-dbildungs-iam-server.json @@ -203,7 +203,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -418,7 +419,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -655,7 +657,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -884,7 +887,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -1105,7 +1109,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -1374,7 +1379,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -1739,7 +1745,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -2509,7 +2516,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -2802,7 +2810,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -3073,7 +3082,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -3465,7 +3475,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -3698,7 +3709,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -3762,7 +3774,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ], "mappedType": "enum" } @@ -3940,7 +3953,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -4251,7 +4265,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -4585,7 +4600,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -4815,7 +4831,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -5091,7 +5108,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { @@ -5259,7 +5277,8 @@ "PERSON_SYNCHRONISIEREN", "CRON_DURCHFUEHREN", "PERSONEN_ANLEGEN", - "IMPORT_DURCHFUEHREN" + "IMPORT_DURCHFUEHREN", + "PERSONEN_LESEN" ] }, "service_provider_target_enum": { diff --git a/migrations/Migration20241207094524-S.ts b/migrations/Migration20241207094524-S.ts new file mode 100644 index 000000000..4a66b3128 --- /dev/null +++ b/migrations/Migration20241207094524-S.ts @@ -0,0 +1,8 @@ +import { Migration } from '@mikro-orm/migrations'; + +export class Migration20241207094524 extends Migration { + async up(): Promise { + this.addSql('alter type "rollen_system_recht_enum" add value if not exists \'PERSONEN_LESEN\';'); + this.addSql("update rolle set ist_technisch = true where name = 'Technical User NextCloud';"); + } +} diff --git a/seeding/dev/01/04_rolle.json b/seeding/dev/01/04_rolle.json index 44d7d81ab..4b95b942f 100644 --- a/seeding/dev/01/04_rolle.json +++ b/seeding/dev/01/04_rolle.json @@ -191,6 +191,16 @@ ], "serviceProviderIds": [], "istTechnisch": true + }, + { + "id": 102, + "administeredBySchulstrukturknoten": 0, + "name": "Technical User NextCloud", + "rollenart": "SYSADMIN", + "merkmale": [], + "systemrechte": ["PERSONEN_LESEN"], + "serviceProviderIds": [], + "istTechnisch": true } ] } diff --git a/seeding/dev/01/05_technical-user.json b/seeding/dev/01/05_technical-user.json index da711613e..01ab7d023 100644 --- a/seeding/dev/01/05_technical-user.json +++ b/seeding/dev/01/05_technical-user.json @@ -6,8 +6,14 @@ "username": "cron-runner", "vorname": "cron-runner", "familienname": "cron-runner", - "keycloakUserId": "7baf74aa-565f-4cfc-9d5a-8f1a3f374dc9", - "personalnummer": "7777777" + "keycloakUserId": "7baf74aa-565f-4cfc-9d5a-8f1a3f374dc9" + }, + { + "id": 7777778, + "username": "nextcloud", + "vorname": "nextcloud", + "password": "SPSHnextcloud1!", + "familienname": "nextcloud" } ] } diff --git a/seeding/dev/01/06_personenkontext.json b/seeding/dev/01/06_personenkontext.json index 29d244cd2..46955ecf1 100644 --- a/seeding/dev/01/06_personenkontext.json +++ b/seeding/dev/01/06_personenkontext.json @@ -545,6 +545,11 @@ "personId": 60, "organisationId": 702, "rolleId": 1 + }, + { + "personId": 7777778, + "organisationId": 0, + "rolleId": 102 } ] } diff --git a/seeding/prod/01/04_rolle.json b/seeding/prod/01/04_rolle.json index 44d7d81ab..4b95b942f 100644 --- a/seeding/prod/01/04_rolle.json +++ b/seeding/prod/01/04_rolle.json @@ -191,6 +191,16 @@ ], "serviceProviderIds": [], "istTechnisch": true + }, + { + "id": 102, + "administeredBySchulstrukturknoten": 0, + "name": "Technical User NextCloud", + "rollenart": "SYSADMIN", + "merkmale": [], + "systemrechte": ["PERSONEN_LESEN"], + "serviceProviderIds": [], + "istTechnisch": true } ] } diff --git a/seeding/prod/01/05_technical-user.json b/seeding/prod/01/05_technical-user.json index da711613e..6e8901e0b 100644 --- a/seeding/prod/01/05_technical-user.json +++ b/seeding/prod/01/05_technical-user.json @@ -6,8 +6,13 @@ "username": "cron-runner", "vorname": "cron-runner", "familienname": "cron-runner", - "keycloakUserId": "7baf74aa-565f-4cfc-9d5a-8f1a3f374dc9", - "personalnummer": "7777777" + "keycloakUserId": "7baf74aa-565f-4cfc-9d5a-8f1a3f374dc9" + }, + { + "id": 7777778, + "username": "nextcloud", + "vorname": "nextcloud", + "familienname": "nextcloud" } ] } diff --git a/seeding/prod/01/06_personenkontext.json b/seeding/prod/01/06_personenkontext.json index 902bbd86b..5643b99a8 100644 --- a/seeding/prod/01/06_personenkontext.json +++ b/seeding/prod/01/06_personenkontext.json @@ -15,7 +15,11 @@ "personId": 7777777, "organisationId": 0, "rolleId": 100 + }, + { + "personId": 7777778, + "organisationId": 0, + "rolleId": 102 } ] } - diff --git a/src/console/dbseed/domain/db-seed.service.spec.ts b/src/console/dbseed/domain/db-seed.service.spec.ts index d16d37ace..9499bac91 100644 --- a/src/console/dbseed/domain/db-seed.service.spec.ts +++ b/src/console/dbseed/domain/db-seed.service.spec.ts @@ -44,6 +44,7 @@ describe('DbSeedService', () => { let personenkontextServiceMock: DeepMocked; let dbSeedReferenceRepoMock: DeepMocked; let kcUserService: DeepMocked; + let personFactory: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -108,6 +109,7 @@ describe('DbSeedService', () => { personenkontextServiceMock = module.get(DBiamPersonenkontextService); dbSeedReferenceRepoMock = module.get(DbSeedReferenceRepo); kcUserService = module.get(KeycloakUserService); + personFactory = module.get(PersonFactory); }); afterAll(async () => { @@ -467,15 +469,34 @@ describe('DbSeedService', () => { 'utf-8', ); - const person: Person = createMock>(); - - personRepoMock.create.mockResolvedValue(person); + const person: Person = createMock>(); + const personPersisted: Person = createMock>(); + personFactory.createNew.mockResolvedValueOnce(person); + personRepoMock.create.mockResolvedValue(personPersisted); await dbSeedService.seedTechnicalUser(fileContentAsStr); expect(dbSeedReferenceRepoMock.create).toHaveBeenCalledTimes(0); }); }); + + describe('error in person factory', () => { + it('should throw Domain Error', async () => { + const fileContentAsStr: string = fs.readFileSync( + `./seeding/seeding-integration-test/invalidPerson/06_technical-user.json`, + 'utf-8', + ); + + // return DomainError + personFactory.createNew.mockResolvedValueOnce(new NameForOrganisationWithTrailingSpaceError()); + + await expect(dbSeedService.seedTechnicalUser(fileContentAsStr)).rejects.toThrow( + NameForOrganisationWithTrailingSpaceError, + ); + expect(personRepoMock.create).toHaveBeenCalledTimes(0); + expect(dbSeedReferenceRepoMock.create).toHaveBeenCalledTimes(0); + }); + }); }); describe('seedPersonkontext', () => { diff --git a/src/console/dbseed/domain/db-seed.service.ts b/src/console/dbseed/domain/db-seed.service.ts index fa29d6df3..7b358cb2e 100644 --- a/src/console/dbseed/domain/db-seed.service.ts +++ b/src/console/dbseed/domain/db-seed.service.ts @@ -308,22 +308,27 @@ export class DbSeedService { /* eslint-disable no-await-in-loop */ for (const file of files) { /* eslint-disable no-await-in-loop */ - const person: Person = Person.construct( - undefined, - undefined, - undefined, - file.familienname, - file.vorname, - '1', - file.username, - file.keycloakUserId, - ); + const creationParams: PersonCreationParams = { + familienname: file.familienname, + vorname: file.vorname, + username: file.username, + password: file.password, + }; + + const person: Person | DomainError = await this.personFactory.createNew(creationParams); + + if (person instanceof DomainError) { + this.logger.error('Could not create technical user:'); + this.logger.error(JSON.stringify(person)); + throw person; + } + + person.keycloakUserId = file.keycloakUserId; const persistedPerson: Person | DomainError = await this.personRepository.create( person, undefined, this.getValidUuidOrUndefined(file.overrideId), - true, ); if (persistedPerson instanceof Person && file.id != null) { const dbSeedReference: DbSeedReference = DbSeedReference.createNew( diff --git a/src/modules/authentication/domain/person-permissions.spec.ts b/src/modules/authentication/domain/person-permissions.spec.ts index c38f4810c..d390f5838 100644 --- a/src/modules/authentication/domain/person-permissions.spec.ts +++ b/src/modules/authentication/domain/person-permissions.spec.ts @@ -240,6 +240,46 @@ describe('PersonPermissions', () => { expect(permittedOrgas.orgaIds).toContain('1'); expect(permittedOrgas.orgaIds).not.toContain('2'); }); + + it('should return organisations when matching one systemrecht', async () => { + const person: Person = Person.construct( + faker.string.uuid(), + faker.date.past(), + faker.date.recent(), + faker.person.lastName(), + faker.person.firstName(), + '1', + faker.lorem.word(), + undefined, + faker.string.uuid(), + ); + + dbiamPersonenkontextRepoMock.hasSystemrechtAtOrganisation.mockResolvedValue(false); + + const personenkontexte: Personenkontext[] = [createPersonenkontext()]; + dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); + const rolle: Rolle = createMock>(); + rolle.systemrechte = [RollenSystemRecht.PERSONEN_LESEN]; + rolleRepoMock.findByIds.mockResolvedValueOnce(new Map>([['1', rolle]])); + + const personPermissions: PersonPermissions = new PersonPermissions( + dbiamPersonenkontextRepoMock, + organisationRepoMock, + rolleRepoMock, + person, + ); + + const permittedOrgas: PermittedOrgas = await personPermissions.getOrgIdsWithSystemrecht( + [RollenSystemRecht.PERSONEN_VERWALTEN, RollenSystemRecht.PERSONEN_LESEN], + false, + false, + ); + if (permittedOrgas.all) { + fail('permittedOrgas.all should be false'); + } + expect(permittedOrgas.orgaIds).toContain('1'); + expect(permittedOrgas.orgaIds).not.toContain('2'); + }); }); describe('getPersonenkontextewithRoles', () => { @@ -315,6 +355,36 @@ describe('PersonPermissions', () => { expect(result).toBeTruthy(); expect(dbiamPersonenkontextRepoMock.hasSystemrechtAtOrganisation).toHaveBeenCalledTimes(1); }); + + it('should return true if person has one of the systemrechte', async () => { + const person: Person = Person.construct( + faker.string.uuid(), + faker.date.past(), + faker.date.recent(), + faker.person.lastName(), + faker.person.firstName(), + '1', + faker.lorem.word(), + undefined, + faker.string.uuid(), + ); + dbiamPersonenkontextRepoMock.hasSystemrechtAtOrganisation.mockResolvedValueOnce(false); + dbiamPersonenkontextRepoMock.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); + + const personPermissions: PersonPermissions = new PersonPermissions( + dbiamPersonenkontextRepoMock, + organisationRepoMock, + rolleRepoMock, + person, + ); + const result: boolean = await personPermissions.hasSystemrechteAtOrganisation( + '2', + [RollenSystemRecht.PERSONEN_VERWALTEN, RollenSystemRecht.PERSONEN_LESEN], + false, + ); + expect(result).toBeTruthy(); + expect(dbiamPersonenkontextRepoMock.hasSystemrechtAtOrganisation).toHaveBeenCalledTimes(2); + }); }); describe('hasSystemrechteAtRootOrganisation', () => { diff --git a/src/modules/authentication/domain/person-permissions.ts b/src/modules/authentication/domain/person-permissions.ts index 00364a79c..bdb7e58fd 100644 --- a/src/modules/authentication/domain/person-permissions.ts +++ b/src/modules/authentication/domain/person-permissions.ts @@ -67,8 +67,9 @@ export class PersonPermissions implements IPersonPermissions { public async getOrgIdsWithSystemrecht( systemrechte: RollenSystemRecht[], withChildren: boolean = false, + matchAll: boolean = true, ): Promise { - if (await this.hasSystemrechteAtRootOrganisation(systemrechte)) { + if (await this.hasSystemrechteAtRootOrganisation(systemrechte, matchAll)) { return { all: true }; } const organisationIDs: Set = new Set(); @@ -80,7 +81,12 @@ export class PersonPermissions implements IPersonPermissions { for (const pk of personKontextFields) { const rolle: Rolle | undefined = rollen.get(pk.rolleId); - if (rolle && systemrechte.every((r: RollenSystemRecht) => rolle.hasSystemRecht(r))) { + if ( + rolle && + (matchAll + ? systemrechte.every((r: RollenSystemRecht) => rolle.hasSystemRecht(r)) + : systemrechte.some((r: RollenSystemRecht) => rolle.hasSystemRecht(r))) + ) { organisationIDs.add(pk.organisationId); } } @@ -102,6 +108,7 @@ export class PersonPermissions implements IPersonPermissions { public async hasSystemrechteAtOrganisation( organisationId: OrganisationID, systemrechte: RollenSystemRecht[], + matchAll: boolean = true, ): Promise { const checks: Promise[] = systemrechte.map( (systemrecht: RollenSystemRecht): Promise => @@ -109,11 +116,16 @@ export class PersonPermissions implements IPersonPermissions { ); const results: boolean[] = await Promise.all(checks); - return results.every((result: boolean): boolean => result); + return matchAll + ? results.every((result: boolean): boolean => result) + : results.some((result: boolean): boolean => result); } - public async hasSystemrechteAtRootOrganisation(systemrechte: RollenSystemRecht[]): Promise { - return this.hasSystemrechteAtOrganisation(this.organisationRepo.ROOT_ORGANISATION_ID, systemrechte); + public async hasSystemrechteAtRootOrganisation( + systemrechte: RollenSystemRecht[], + matchAll: boolean = true, + ): Promise { + return this.hasSystemrechteAtOrganisation(this.organisationRepo.ROOT_ORGANISATION_ID, systemrechte, matchAll); } public async hasSystemrechtAtOrganisation( diff --git a/src/modules/person/api/person.controller.ts b/src/modules/person/api/person.controller.ts index c0c565335..6df2fd834 100644 --- a/src/modules/person/api/person.controller.ts +++ b/src/modules/person/api/person.controller.ts @@ -301,8 +301,9 @@ export class PersonController { ): Promise> { // Find all organisations where user has permission const permittedOrgas: PermittedOrgas = await permissions.getOrgIdsWithSystemrecht( - [RollenSystemRecht.PERSONEN_VERWALTEN], + [RollenSystemRecht.PERSONEN_VERWALTEN, RollenSystemRecht.PERSONEN_LESEN], true, + false, ); // Find all Personen on child-orgas (+root orgas) diff --git a/src/modules/person/persistence/person.repository.integration-spec.ts b/src/modules/person/persistence/person.repository.integration-spec.ts index 4ab102d5d..813d49d2d 100644 --- a/src/modules/person/persistence/person.repository.integration-spec.ts +++ b/src/modules/person/persistence/person.repository.integration-spec.ts @@ -300,7 +300,7 @@ describe('PersonRepository Integration', () => { describe('create', () => { describe('When Normal Call Without Hashed Password', () => { describe('when person has already keycloak user', () => { - it('should return Domain Error', async () => { + it('should return Person', async () => { usernameGeneratorService.generateUsername.mockResolvedValueOnce({ ok: true, value: 'testusername', @@ -314,21 +314,10 @@ describe('PersonRepository Integration', () => { throw person; } person.keycloakUserId = faker.string.uuid(); - kcUserServiceMock.create.mockResolvedValueOnce({ - ok: true, - value: '', - }); - kcUserServiceMock.setPassword.mockResolvedValueOnce({ - ok: true, - value: '', - }); - kcUserServiceMock.delete.mockResolvedValueOnce({ - ok: true, - value: undefined, - }); + const result: Person | DomainError = await sut.create(person); - expect(result).toBeInstanceOf(DomainError); + expect(result).toBeInstanceOf(Person); expect(kcUserServiceMock.create).not.toHaveBeenCalled(); }); }); diff --git a/src/modules/person/persistence/person.repository.ts b/src/modules/person/persistence/person.repository.ts index 2a44931d3..f0592690d 100644 --- a/src/modules/person/persistence/person.repository.ts +++ b/src/modules/person/persistence/person.repository.ts @@ -357,7 +357,6 @@ export class PersonRepository { person: Person, hashedPassword?: string, personId?: string, - technicalUser: boolean = false, ): Promise | DomainError> { const transaction: EntityManager = this.em.fork(); await transaction.begin(); @@ -386,7 +385,7 @@ export class PersonRepository { // Take ID from person to create keycloak user let personWithKeycloakUser: Person | DomainError; - if (!technicalUser) { + if (!person.keycloakUserId) { if (!hashedPassword) { personWithKeycloakUser = await this.createKeycloakUser(persistedPerson, this.kcUserService); } else { diff --git a/src/modules/personenkontext/api/personenkontext.controller.spec.ts b/src/modules/personenkontext/api/personenkontext.controller.spec.ts index ef24dc058..e8fdf61c5 100644 --- a/src/modules/personenkontext/api/personenkontext.controller.spec.ts +++ b/src/modules/personenkontext/api/personenkontext.controller.spec.ts @@ -260,8 +260,9 @@ describe('PersonenkontextController', () => { ); expect(permissionsMock.getOrgIdsWithSystemrecht).toHaveBeenCalledWith( - [RollenSystemRecht.PERSONEN_VERWALTEN], + [RollenSystemRecht.PERSONEN_VERWALTEN, RollenSystemRecht.PERSONEN_LESEN], true, + false, ); expect(result.items.length).toBe(1); if (result.items[0]) { @@ -302,8 +303,9 @@ describe('PersonenkontextController', () => { ); expect(permissionsMock.getOrgIdsWithSystemrecht).toHaveBeenCalledWith( - [RollenSystemRecht.PERSONEN_VERWALTEN], + [RollenSystemRecht.PERSONEN_VERWALTEN, RollenSystemRecht.PERSONEN_LESEN], true, + false, ); expect(result.items.length).toBe(1); if (result.items[0]) { diff --git a/src/modules/personenkontext/api/personenkontext.controller.ts b/src/modules/personenkontext/api/personenkontext.controller.ts index 52daa15d6..ecd092f28 100644 --- a/src/modules/personenkontext/api/personenkontext.controller.ts +++ b/src/modules/personenkontext/api/personenkontext.controller.ts @@ -134,8 +134,9 @@ export class PersonenkontextController { @Permissions() permissions: PersonPermissions, ): Promise> { const permittedOrgas: PermittedOrgas = await permissions.getOrgIdsWithSystemrecht( - [RollenSystemRecht.PERSONEN_VERWALTEN], + [RollenSystemRecht.PERSONEN_VERWALTEN, RollenSystemRecht.PERSONEN_LESEN], true, + false, ); const result: Paged> = await this.personenkontextService.findAllPersonenkontexte( diff --git a/src/modules/rolle/domain/rolle.enums.ts b/src/modules/rolle/domain/rolle.enums.ts index 7a7a07722..825ee1ea3 100644 --- a/src/modules/rolle/domain/rolle.enums.ts +++ b/src/modules/rolle/domain/rolle.enums.ts @@ -28,4 +28,5 @@ export enum RollenSystemRecht { CRON_DURCHFUEHREN = 'CRON_DURCHFUEHREN', PERSONEN_ANLEGEN = 'PERSONEN_ANLEGEN', IMPORT_DURCHFUEHREN = 'IMPORT_DURCHFUEHREN', // Requires PERSONEN_VERWALTEN (later PERSONEN_ERSTELLEN !!!) to work + PERSONEN_LESEN = 'PERSONEN_LESEN', } From 6e50c8f4e1595e725c93032635eb7948d390ce48 Mon Sep 17 00:00:00 2001 From: Youssef Bouchara <101522419+YoussefBouch@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:44:25 +0100 Subject: [PATCH 2/8] Fixed rolle name (#840) --- seeding/dev/01/04_rolle.json | 2 +- seeding/prod/01/04_rolle.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/seeding/dev/01/04_rolle.json b/seeding/dev/01/04_rolle.json index 4b95b942f..cea655a16 100644 --- a/seeding/dev/01/04_rolle.json +++ b/seeding/dev/01/04_rolle.json @@ -131,7 +131,7 @@ { "id": 14, "administeredBySchulstrukturknoten": 0, - "name": "IQSH Mitarbeiter ohne Landes-E-Mail", + "name": "IQSH Mitarbeiter", "rollenart": "LEHR", "merkmale": ["KOPERS_PFLICHT"], "systemrechte": [], diff --git a/seeding/prod/01/04_rolle.json b/seeding/prod/01/04_rolle.json index 4b95b942f..cea655a16 100644 --- a/seeding/prod/01/04_rolle.json +++ b/seeding/prod/01/04_rolle.json @@ -131,7 +131,7 @@ { "id": 14, "administeredBySchulstrukturknoten": 0, - "name": "IQSH Mitarbeiter ohne Landes-E-Mail", + "name": "IQSH Mitarbeiter", "rollenart": "LEHR", "merkmale": ["KOPERS_PFLICHT"], "systemrechte": [], From 0c085437b0c62e24d258a27b651c225bab8b76b4 Mon Sep 17 00:00:00 2001 From: Cornelius <144817755+DPDS93CT@users.noreply.github.com> Date: Thu, 12 Dec 2024 08:25:53 +0100 Subject: [PATCH 3/8] =?UTF-8?q?SPSH-1584:=20OX:=20E-Mail=20Adresse=20bei?= =?UTF-8?q?=20Namens=C3=A4nderung=20wird=20nicht=20angepasst=20(#826)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * adjust OXUserChange (renaming user) to set referrer as OxUserName also adjust evnet regarding ID_OX in KC * expect more info-logging in test cases for OxEventHandler --- .../ox/domain/ox-event-handler.spec.ts | 18 +++++++++++++----- src/modules/ox/domain/ox-event-handler.ts | 19 +++++++++++++------ 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/modules/ox/domain/ox-event-handler.spec.ts b/src/modules/ox/domain/ox-event-handler.spec.ts index 0513f6411..58cdd885e 100644 --- a/src/modules/ox/domain/ox-event-handler.spec.ts +++ b/src/modules/ox/domain/ox-event-handler.spec.ts @@ -546,7 +546,7 @@ describe('OxEventHandler', () => { expect(oxServiceMock.send).toHaveBeenCalledTimes(0); expect(loggerMock.error).toHaveBeenLastCalledWith( - `No requested email-address found for personId:${personId}`, + `No REQUESTED email-address found for personId:${personId}`, ); }); @@ -828,7 +828,7 @@ describe('OxEventHandler', () => { expect(oxServiceMock.send).toHaveBeenCalledTimes(0); expect(loggerMock.error).toHaveBeenLastCalledWith( - `No requested email-address found for personId:${personId}`, + `No REQUESTED email-address found for personId:${personId}`, ); }); @@ -878,12 +878,12 @@ describe('OxEventHandler', () => { it('should publish OxUserChangedEvent on success', async () => { personRepositoryMock.findById.mockResolvedValueOnce(person); emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce(getRequestedEmailAddresses(email)); - + const currentAliases: string[] = [faker.internet.email()]; //mock getData oxServiceMock.send.mockResolvedValueOnce({ ok: true, value: createMock({ - aliases: [faker.internet.email()], + aliases: currentAliases, username: oxUserName, id: oxUserId, primaryEmail: email, @@ -900,6 +900,14 @@ describe('OxEventHandler', () => { expect(oxServiceMock.send).toHaveBeenCalledTimes(2); expect(loggerMock.error).toHaveBeenCalledTimes(0); + expect(loggerMock.info).toHaveBeenCalledWith( + `Found mostRecentRequested Email-Address:${JSON.stringify(email)} For personId:${personId}`, + ); + //use regex, because strict comparison fails, local test-var currentAliases has changed by the implemented function when expect is checked here + expect(loggerMock.info).toHaveBeenCalledWith( + expect.stringMatching(/Found Current aliases:.* For personId:/), + ); + expect(loggerMock.info).toHaveBeenCalledWith(`Added New alias:${email} For personId:${personId}`); expect(loggerMock.info).toHaveBeenLastCalledWith( `Changed primary email-address in OX for user, username:${person.referrer}, new email-address:${email}`, ); @@ -908,7 +916,7 @@ describe('OxEventHandler', () => { personId: personId, keycloakUsername: referrer, oxUserId: oxUserId, - oxUserName: oxUserName, + oxUserName: referrer, //this is the new OxUserName, it's changed on renaming in SPSH oxContextId: contextId, oxContextName: contextName, primaryEmail: email, diff --git a/src/modules/ox/domain/ox-event-handler.ts b/src/modules/ox/domain/ox-event-handler.ts index 5accc0054..0dd0982f2 100644 --- a/src/modules/ox/domain/ox-event-handler.ts +++ b/src/modules/ox/domain/ox-event-handler.ts @@ -303,9 +303,13 @@ export class OxEventHandler { const requestedEmailAddresses: Option[]> = await this.emailRepo.findByPersonSortedByUpdatedAtDesc(personId, EmailAddressStatus.REQUESTED); if (!requestedEmailAddresses || !requestedEmailAddresses[0]) { - this.logger.error(`No requested email-address found for personId:${personId}`); + this.logger.error(`No REQUESTED email-address found for personId:${personId}`); return undefined; } + this.logger.info( + `Found mostRecentRequested Email-Address:${JSON.stringify(requestedEmailAddresses[0].address)} For personId:${personId}`, + ); + return requestedEmailAddresses[0]; } @@ -615,15 +619,18 @@ export class OxEventHandler { ); } const newAliasesArray: string[] = getDataResult.value.aliases; + this.logger.info(`Found Current aliases:${JSON.stringify(newAliasesArray)} For personId:${personId}`); + newAliasesArray.push(requestedEmailAddressString); + this.logger.info(`Added New alias:${requestedEmailAddressString} For personId:${personId}`); const params: ChangeUserParams = { contextId: this.contextID, - userId: getDataResult.value.id, - username: getDataResult.value.username, + userId: person.oxUserId, + username: person.referrer, givenname: person.vorname, surname: person.familienname, - displayname: person.referrer, + displayname: person.referrer, //IS EXPLICITLY NOT SET to vorname+familienname defaultSenderAddress: requestedEmailAddressString, email1: requestedEmailAddressString, aliases: newAliasesArray, @@ -653,8 +660,8 @@ export class OxEventHandler { new OxUserChangedEvent( personId, person.referrer, - getDataResult.value.id, - getDataResult.value.username, + person.oxUserId, + person.referrer, //strictEquals the new OxUsername this.contextID, this.contextName, requestedEmailAddressString, From 2ef7a259cd3a41ad81b7ee66eed56ecf0c1d9c30 Mon Sep 17 00:00:00 2001 From: Cornelius <144817755+DPDS93CT@users.noreply.github.com> Date: Thu, 12 Dec 2024 09:28:12 +0100 Subject: [PATCH 4/8] SPSH-1592 (#828) * add personID to some logs in OxEventHandler and EmailEventHandler * log warning if multiple REQUESTED emailAddresses can be found for person * log warning when multiple REQUESTED emailAddresses found for personId --- .../email/domain/email-event-handler.spec.ts | 10 ++-- .../email/domain/email-event-handler.ts | 14 +++-- .../email/persistence/email.repo.spec.ts | 56 +++++++++++++++---- src/modules/email/persistence/email.repo.ts | 25 ++++++--- .../ox/domain/ox-event-handler.spec.ts | 4 +- src/modules/ox/domain/ox-event-handler.ts | 2 +- 6 files changed, 80 insertions(+), 31 deletions(-) diff --git a/src/modules/email/domain/email-event-handler.spec.ts b/src/modules/email/domain/email-event-handler.spec.ts index d9137c1c1..738b75acb 100644 --- a/src/modules/email/domain/email-event-handler.spec.ts +++ b/src/modules/email/domain/email-event-handler.spec.ts @@ -56,7 +56,7 @@ function getEmail(): EmailAddress { ); } -describe('Email Event Handler', () => { +describe('EmailEventHandler', () => { let app: INestApplication; let emailEventHandler: EmailEventHandler; @@ -1129,10 +1129,10 @@ describe('Email Event Handler', () => { await emailEventHandler.handleOxMetadataInKeycloakChangedEvent(event); expect(loggerMock.warning).toHaveBeenCalledWith( - `Mismatch between requested(${emailAddress}) and received(${event.emailAddress}) address from OX`, + `Mismatch between requested(${emailAddress}) and received(${event.emailAddress}) address from OX, personId:${event.personId}`, ); expect(loggerMock.warning).toHaveBeenLastCalledWith( - `Overriding ${emailAddress} with ${event.emailAddress}) from OX`, + `Overriding ${emailAddress} with ${event.emailAddress}) from OX, personId:${event.personId}`, ); }); }); @@ -1152,7 +1152,7 @@ describe('Email Event Handler', () => { await emailEventHandler.handleOxMetadataInKeycloakChangedEvent(event); expect(loggerMock.error).toHaveBeenLastCalledWith( - `Could not enable email, error is EmailAddress with ID 1 could not be updated`, + `Could not enable email for personId:${event.personId}, error is EmailAddress with ID 1 could not be updated`, ); }); }); @@ -1171,7 +1171,7 @@ describe('Email Event Handler', () => { await emailEventHandler.handleOxMetadataInKeycloakChangedEvent(event); expect(loggerMock.info).toHaveBeenLastCalledWith( - `Changed email-address:${fakeEmail} from REQUESTED to ENABLED`, + `Changed email-address:${fakeEmail} from REQUESTED to ENABLED, personId:${event.personId}`, ); }); }); diff --git a/src/modules/email/domain/email-event-handler.ts b/src/modules/email/domain/email-event-handler.ts index f1828225b..a572c6160 100644 --- a/src/modules/email/domain/email-event-handler.ts +++ b/src/modules/email/domain/email-event-handler.ts @@ -253,9 +253,11 @@ export class EmailEventHandler { if (email.address !== event.emailAddress) { this.logger.warning( - `Mismatch between requested(${email.address}) and received(${event.emailAddress}) address from OX`, + `Mismatch between requested(${email.address}) and received(${event.emailAddress}) address from OX, personId:${event.personId}`, + ); + this.logger.warning( + `Overriding ${email.address} with ${event.emailAddress}) from OX, personId:${event.personId}`, ); - this.logger.warning(`Overriding ${email.address} with ${event.emailAddress}) from OX`); email.setAddress(event.emailAddress); } @@ -264,9 +266,13 @@ export class EmailEventHandler { const persistenceResult: EmailAddress | DomainError = await this.emailRepo.save(email); if (persistenceResult instanceof DomainError) { - return this.logger.error(`Could not enable email, error is ${persistenceResult.message}`); + return this.logger.error( + `Could not enable email for personId:${event.personId}, error is ${persistenceResult.message}`, + ); } else { - return this.logger.info(`Changed email-address:${persistenceResult.address} from REQUESTED to ENABLED`); + return this.logger.info( + `Changed email-address:${persistenceResult.address} from REQUESTED to ENABLED, personId:${event.personId}`, + ); } } diff --git a/src/modules/email/persistence/email.repo.spec.ts b/src/modules/email/persistence/email.repo.spec.ts index 98722fa20..da0a88c36 100644 --- a/src/modules/email/persistence/email.repo.spec.ts +++ b/src/modules/email/persistence/email.repo.spec.ts @@ -8,7 +8,7 @@ import { } from '../../../../test/utils/index.js'; import { EmailRepo, mapAggregateToData } from './email.repo.js'; import { EmailFactory } from '../domain/email.factory.js'; -import { createMock } from '@golevelup/ts-jest'; +import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { PersonFactory } from '../../person/domain/person.factory.js'; import { PersonRepository } from '../../person/persistence/person.repository.js'; import { UsernameGeneratorService } from '../../person/domain/username-generator.service.js'; @@ -27,6 +27,7 @@ import { PersonAlreadyHasEnabledEmailAddressError } from '../error/person-alread import { UserLockRepository } from '../../keycloak-administration/repository/user-lock.repository.js'; import { PersonEmailResponse } from '../../person/api/person-email-response.js'; import { generatePassword } from '../../../shared/util/password-generator.js'; +import { OrganisationID, PersonID } from '../../../shared/types/aggregate-ids.types.js'; describe('EmailRepo', () => { let module: TestingModule; @@ -37,6 +38,8 @@ describe('EmailRepo', () => { let organisationRepository: OrganisationRepository; let orm: MikroORM; + let loggerMock: DeepMocked; + beforeAll(async () => { module = await Test.createTestingModule({ imports: [ConfigTestModule, DatabaseTestModule.forRoot({ isDatabaseRequired: true })], @@ -83,6 +86,8 @@ describe('EmailRepo', () => { organisationRepository = module.get(OrganisationRepository); orm = module.get(MikroORM); + loggerMock = module.get(ClassLogger); + await DatabaseTestModule.setupDatabase(orm); }, DEFAULT_TIMEOUT_FOR_TESTCONTAINERS); @@ -116,12 +121,12 @@ describe('EmailRepo', () => { return organisationRepository.save(organisation); } - async function createPersonAndOrganisationAndEmailAddress( + async function createEmailAddress( status: EmailAddressStatus, - ): Promise<[Person, Organisation, EmailAddress]> { - const person: Person = await createPerson(); - const organisation: Organisation = await createOrganisation(); - const email: Result> = await emailFactory.createNew(person.id, organisation.id); + personId: PersonID, + organisationId: OrganisationID, + ): Promise> { + const email: Result> = await emailFactory.createNew(personId, organisationId); if (!email.ok) throw new Error(); switch (status) { @@ -135,7 +140,17 @@ describe('EmailRepo', () => { const savedEmail: EmailAddress | DomainError = await sut.save(email.value); if (savedEmail instanceof DomainError) throw new Error(); - return [person, organisation, savedEmail]; + return savedEmail; + } + + async function createPersonAndOrganisationAndEmailAddress( + status: EmailAddressStatus, + ): Promise<[Person, Organisation, EmailAddress]> { + const person: Person = await createPerson(); + const organisation: Organisation = await createOrganisation(); + const email: EmailAddress = await createEmailAddress(status, person.id, organisation.id); + + return [person, organisation, email]; } afterAll(async () => { @@ -175,7 +190,7 @@ describe('EmailRepo', () => { describe('findRequestedByPerson', () => { describe('when email-address is found for personId', () => { - it('should return email with email-addresses by personId', async () => { + it('should return emailAddress by personId', async () => { const [person, ,]: [Person, Organisation, EmailAddress] = await createPersonAndOrganisationAndEmailAddress(EmailAddressStatus.REQUESTED); @@ -186,11 +201,32 @@ describe('EmailRepo', () => { }); }); + describe('when multiple email-addresses are found for personId', () => { + it('should return most recently updated emailAddress by personId and log warning', async () => { + const [person, organisation]: [Person, Organisation, EmailAddress] = + await createPersonAndOrganisationAndEmailAddress(EmailAddressStatus.REQUESTED); + const newerEmailAddress: EmailAddress = await createEmailAddress( + EmailAddressStatus.REQUESTED, + person.id, + organisation.id, + ); + + const foundEmail: Option> = await sut.findRequestedByPerson(person.id); + if (!foundEmail) throw Error(); + + expect(loggerMock.warning).toHaveBeenCalledWith( + `Multiple EmailAddresses Found In REQUESTED Status For personId:${person.id}, Will Only Return address:${newerEmailAddress.address}`, + ); + + expect(foundEmail.address).toStrictEqual(newerEmailAddress.address); + }); + }); + describe('when person does NOT exist', () => { - it('should return undefined', async () => { + it('should return null', async () => { const foundEmail: Option> = await sut.findRequestedByPerson(faker.string.uuid()); - expect(foundEmail).toBeUndefined(); + expect(foundEmail).toBeNull(); }); }); }); diff --git a/src/modules/email/persistence/email.repo.ts b/src/modules/email/persistence/email.repo.ts index efd7ac462..51bc423a6 100644 --- a/src/modules/email/persistence/email.repo.ts +++ b/src/modules/email/persistence/email.repo.ts @@ -56,18 +56,25 @@ export class EmailRepo { return mapEntityToAggregate(emailAddressEntity); } + /** + * Will return the most recently updated EmailAddress if multiple EmailAddresses with REQUESTED status can be found for personId, warning will be logged in this case. + * @param personId + */ public async findRequestedByPerson(personId: PersonID): Promise>> { - const emailAddressEntity: Option = await this.em.findOne( - EmailAddressEntity, - { - personId: { $eq: personId }, - status: { $eq: EmailAddressStatus.REQUESTED }, - }, - {}, + const emailAddresses: EmailAddress[] = await this.findByPersonSortedByUpdatedAtDesc( + personId, + EmailAddressStatus.REQUESTED, ); - if (!emailAddressEntity) return undefined; - return mapEntityToAggregate(emailAddressEntity); + if (!emailAddresses || !emailAddresses[0]) return null; + + if (emailAddresses.length > 1) { + this.logger.warning( + `Multiple EmailAddresses Found In REQUESTED Status For personId:${personId}, Will Only Return address:${emailAddresses[0].address}`, + ); + } + + return emailAddresses[0]; } public async findByPersonSortedByUpdatedAtDesc( diff --git a/src/modules/ox/domain/ox-event-handler.spec.ts b/src/modules/ox/domain/ox-event-handler.spec.ts index 58cdd885e..661c119b2 100644 --- a/src/modules/ox/domain/ox-event-handler.spec.ts +++ b/src/modules/ox/domain/ox-event-handler.spec.ts @@ -599,7 +599,7 @@ describe('OxEventHandler', () => { expect(oxServiceMock.send).toHaveBeenCalledWith(expect.any(CreateUserAction)); expect(loggerMock.info).toHaveBeenCalledWith( - `User created in OX, userId:${fakeOXUserId}, email:${event.address}`, + `User created in OX, oxUserId:${fakeOXUserId}, oxEmail:${event.address}, personId:${personId}`, ); expect(loggerMock.info).toHaveBeenLastCalledWith( `Successfully Added OxUser To OxGroup, oxUserId:${fakeOXUserId}, oxGroupId:${fakeOXGroupId}`, @@ -720,7 +720,7 @@ describe('OxEventHandler', () => { expect(oxServiceMock.send).toHaveBeenCalledWith(expect.any(CreateUserAction)); expect(loggerMock.info).toHaveBeenLastCalledWith( - `User created in OX, userId:${fakeOXUserId}, email:${event.address}`, + `User created in OX, oxUserId:${fakeOXUserId}, oxEmail:${event.address}, personId:${personId}`, ); expect(loggerMock.error).toHaveBeenLastCalledWith( `Persisting oxUserId on emailAddress for personId:${personId} failed`, diff --git a/src/modules/ox/domain/ox-event-handler.ts b/src/modules/ox/domain/ox-event-handler.ts index 0dd0982f2..9183ad28e 100644 --- a/src/modules/ox/domain/ox-event-handler.ts +++ b/src/modules/ox/domain/ox-event-handler.ts @@ -512,7 +512,7 @@ export class OxEventHandler { } this.logger.info( - `User created in OX, userId:${createUserResult.value.id}, email:${createUserResult.value.primaryEmail}`, + `User created in OX, oxUserId:${createUserResult.value.id}, oxEmail:${createUserResult.value.primaryEmail}, personId:${personId}`, ); mostRecentRequestedEmailAddress.oxUserID = createUserResult.value.id; From 8f74727486d337a8467ddf812cdd53da4fdfa8d7 Mon Sep 17 00:00:00 2001 From: Cornelius <144817755+DPDS93CT@users.noreply.github.com> Date: Thu, 12 Dec 2024 10:11:47 +0100 Subject: [PATCH 5/8] SPSH-1544: UEM Inbetriebnahme-Passwort erzeugen (#820) * rm PersonRepository-ref in LdapClientSvc, adjust Email-events to contain referrer, add resetUEMPasswordByPersonId in PersonController, add PersonReferrer-type * fix existing test cases * add test cases after adjusting LDAPClientSvc * remove dead imports and comments * add RollenArt to DBiamPersonenzuordnungResponse * remove StepUp-Guard for Reset-UEM-password * allow Lehrer to use UEM-password-reset for themselves * undo StepUpGuard-disabling for creating person with kontexte via POST * adjust i18 for Person-UEM-Password, adjust test cases --------- Co-authored-by: Youssef Bouchara <101522419+YoussefBouch@users.noreply.github.com> --- .../ldap/domain/ldap-client.service.spec.ts | 150 +++++++++++++++--- src/core/ldap/domain/ldap-client.service.ts | 82 +++++++--- .../ldap/domain/ldap-event-handler.spec.ts | 2 + src/core/ldap/domain/ldap-event-handler.ts | 4 +- .../error/ldap-modify-user-password.error.ts | 7 + src/core/ldap/ldap.module.ts | 11 +- .../email/domain/email-event-handler.spec.ts | 103 +++++++++++- .../email/domain/email-event-handler.ts | 41 +++++ .../ox/domain/ox-event-handler.spec.ts | 5 + src/modules/person/api/dbiam-person.error.ts | 1 + .../person/api/person-exception-filter.ts | 8 + .../person/api/person.controller.spec.ts | 133 +++++++++++++++- src/modules/person/api/person.controller.ts | 47 +++++- .../dbiam-personenzuordnung.response.ts | 6 +- ...person-user-password-modification.error.ts | 7 + .../person.repository.integration-spec.ts | 116 ++++++++++++++ .../person/persistence/person.repository.ts | 14 ++ src/modules/person/person-api.module.ts | 2 + .../events/email-address-changed.event.ts | 3 +- .../events/email-address-generated.event.ts | 3 +- .../events/ldap-person-entry-changed.event.ts | 1 + src/shared/types/aggregate-ids.types.ts | 3 + 22 files changed, 680 insertions(+), 69 deletions(-) create mode 100644 src/core/ldap/error/ldap-modify-user-password.error.ts create mode 100644 src/modules/person/domain/person-user-password-modification.error.ts diff --git a/src/core/ldap/domain/ldap-client.service.spec.ts b/src/core/ldap/domain/ldap-client.service.spec.ts index 19015a2a2..de02b8202 100644 --- a/src/core/ldap/domain/ldap-client.service.spec.ts +++ b/src/core/ldap/domain/ldap-client.service.spec.ts @@ -27,8 +27,8 @@ 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'; -import { PersonRepository } from '../../../modules/person/persistence/person.repository.js'; import { LdapInstanceConfig } from '../ldap-instance-config.js'; +import { LdapModifyUserPasswordError } from '../error/ldap-modify-user-password.error.js'; describe('LDAP Client Service', () => { let app: INestApplication; @@ -40,7 +40,6 @@ describe('LDAP Client Service', () => { let loggerMock: DeepMocked; let eventServiceMock: DeepMocked; let clientMock: DeepMocked; - let personRepoMock: DeepMocked; let instanceConfig: LdapInstanceConfig; let person: Person; @@ -69,8 +68,6 @@ describe('LDAP Client Service', () => { .useValue(createMock()) .overrideProvider(EventService) .useValue(createMock()) - .overrideProvider(PersonRepository) - .useValue(createMock()) .compile(); orm = module.get(MikroORM); @@ -80,7 +77,6 @@ describe('LDAP Client Service', () => { loggerMock = module.get(ClassLogger); eventServiceMock = module.get(EventService); clientMock = createMock(); - personRepoMock = module.get(PersonRepository); instanceConfig = module.get(LdapInstanceConfig); person = Person.construct( @@ -459,7 +455,6 @@ describe('LDAP Client Service', () => { clientMock.del.mockResolvedValueOnce(); return clientMock; }); - personRepoMock.findById.mockResolvedValueOnce(person); const result: Result = await ldapClientService.deleteLehrerByReferrer(person.referrer!); @@ -613,13 +608,13 @@ describe('LDAP Client Service', () => { describe('when bind returns error', () => { it('should return falsy result', async () => { - personRepoMock.findById.mockResolvedValueOnce(person); ldapClientMock.getClient.mockImplementation(() => { clientMock.bind.mockRejectedValueOnce(new Error()); return clientMock; }); const result: Result = await ldapClientService.changeEmailAddressByPersonId( faker.string.uuid(), + faker.internet.userName(), fakeSchuleSHAddress, ); @@ -629,9 +624,9 @@ describe('LDAP Client Service', () => { describe('when person can not be found in DB', () => { it('should return falsy result', async () => { - personRepoMock.findById.mockResolvedValueOnce(undefined); const result: Result = await ldapClientService.changeEmailAddressByPersonId( faker.string.uuid(), + faker.internet.userName(), fakeSchuleSHAddress, ); @@ -641,10 +636,9 @@ describe('LDAP Client Service', () => { describe('when called with invalid emailDomain', () => { it('should return LdapEmailDomainError', async () => { - personRepoMock.findById.mockResolvedValueOnce(person); - const result: Result = await ldapClientService.changeEmailAddressByPersonId( faker.string.uuid(), + faker.internet.userName(), 'user@wrong-email-domain.de', ); @@ -656,10 +650,9 @@ describe('LDAP Client Service', () => { describe('when called with newEmailAddress that is not splittable', () => { it('should return LdapEmailAddressError', async () => { - personRepoMock.findById.mockResolvedValueOnce(person); - const result: Result = await ldapClientService.changeEmailAddressByPersonId( faker.string.uuid(), + faker.internet.userName(), 'user-at-wrong-email-domain.de', ); @@ -671,7 +664,6 @@ describe('LDAP Client Service', () => { describe('when person cannot be found by personID', () => { it('should return LdapSearchError', async () => { - personRepoMock.findById.mockResolvedValueOnce(person); ldapClientMock.getClient.mockImplementation(() => { clientMock.bind.mockResolvedValueOnce(); clientMock.search.mockResolvedValueOnce( @@ -684,6 +676,7 @@ describe('LDAP Client Service', () => { const result: Result = await ldapClientService.changeEmailAddressByPersonId( faker.string.uuid(), + faker.internet.userName(), fakeSchuleSHAddress, ); @@ -702,8 +695,6 @@ describe('LDAP Client Service', () => { const currentEmailAddress: string = 'current-address@schule-sh.de'; it('should set mailAlternativeAddress as current mailPrimaryAddress and throw LdapPersonEntryChangedEvent', async () => { - personRepoMock.findById.mockResolvedValueOnce(person); - ldapClientMock.getClient.mockImplementation(() => { clientMock.bind.mockResolvedValueOnce(); clientMock.search.mockResolvedValueOnce( @@ -723,6 +714,7 @@ describe('LDAP Client Service', () => { const result: Result = await ldapClientService.changeEmailAddressByPersonId( fakePersonID, + faker.internet.userName(), newEmailAddress, ); @@ -750,8 +742,6 @@ describe('LDAP Client Service', () => { describe('and already has a mailPrimaryAddress', () => { it('should set mailAlternativeAddress as current mailPrimaryAddress and throw LdapPersonEntryChangedEvent', async () => { - personRepoMock.findById.mockResolvedValueOnce(person); - ldapClientMock.getClient.mockImplementation(() => { clientMock.bind.mockResolvedValueOnce(); clientMock.search.mockResolvedValueOnce( @@ -771,6 +761,7 @@ describe('LDAP Client Service', () => { const result: Result = await ldapClientService.changeEmailAddressByPersonId( fakePersonID, + faker.internet.userName(), newEmailAddress, ); @@ -798,8 +789,6 @@ describe('LDAP Client Service', () => { describe('and already has a mailPrimaryAddress', () => { it('should set mailAlternativeAddress as current mailPrimaryAddress and throw LdapPersonEntryChangedEvent', async () => { - personRepoMock.findById.mockResolvedValueOnce(person); - ldapClientMock.getClient.mockImplementation(() => { clientMock.bind.mockResolvedValueOnce(); clientMock.search.mockResolvedValueOnce( @@ -819,6 +808,7 @@ describe('LDAP Client Service', () => { const result: Result = await ldapClientService.changeEmailAddressByPersonId( fakePersonID, + faker.internet.userName(), newEmailAddress, ); @@ -839,8 +829,6 @@ describe('LDAP Client Service', () => { describe('but does NOT have a mailPrimaryAddress', () => { it('should set mailAlternativeAddress to same value as mailPrimaryAddress and throw LdapPersonEntryChangedEvent', async () => { - personRepoMock.findById.mockResolvedValueOnce(person); - ldapClientMock.getClient.mockImplementation(() => { clientMock.bind.mockResolvedValueOnce(); clientMock.search.mockResolvedValueOnce( @@ -860,6 +848,7 @@ describe('LDAP Client Service', () => { const result: Result = await ldapClientService.changeEmailAddressByPersonId( fakePersonID, + faker.internet.userName(), newEmailAddress, ); @@ -878,4 +867,123 @@ describe('LDAP Client Service', () => { }); }); }); + + describe('changeUserPasswordByPersonId', () => { + describe('when bind returns error', () => { + it('should return falsy result', async () => { + ldapClientMock.getClient.mockImplementation(() => { + clientMock.bind.mockRejectedValueOnce(new Error()); + return clientMock; + }); + const result: Result = await ldapClientService.changeUserPasswordByPersonId( + faker.string.uuid(), + faker.internet.userName(), + ); + + expect(result.ok).toBeFalsy(); + }); + }); + + describe('when person cannot be found by personID', () => { + it('should return LdapSearchError', async () => { + ldapClientMock.getClient.mockImplementation(() => { + clientMock.bind.mockResolvedValueOnce(); + clientMock.search.mockResolvedValueOnce( + createMock({ + searchEntries: [], + }), + ); + return clientMock; + }); + + const result: Result = await ldapClientService.changeUserPasswordByPersonId( + faker.string.uuid(), + faker.internet.userName(), + ); + + expect(result.ok).toBeFalsy(); + expect(result).toEqual({ + ok: false, + error: new LdapSearchError(LdapEntityType.LEHRER), + }); + }); + }); + + describe('when person can be found but modification fails', () => { + const fakePersonID: string = faker.string.uuid(); + const fakeDN: string = faker.string.alpha(); + + it('should NOT publish event and throw LdapPersonEntryChangedEvent', async () => { + ldapClientMock.getClient.mockImplementation(() => { + clientMock.bind.mockResolvedValueOnce(); + clientMock.search.mockResolvedValueOnce( + createMock({ + searchEntries: [ + createMock({ + dn: fakeDN, + }), + ], + }), + ); + clientMock.modify.mockRejectedValueOnce(new Error()); + + return clientMock; + }); + + const result: Result = await ldapClientService.changeUserPasswordByPersonId( + fakePersonID, + faker.internet.userName(), + ); + + if (result.ok) throw Error(); + expect(result.error).toStrictEqual(new LdapModifyUserPasswordError()); + expect(loggerMock.error).toHaveBeenLastCalledWith( + `LDAP: Modifying userPassword (UEM) FAILED, errMsg:{}`, + ); + expect(eventServiceMock.publish).toHaveBeenCalledTimes(0); + }); + }); + + describe('when person can be found and userPassword can be modified', () => { + let fakePersonID: string; + let fakeDN: string; + + beforeEach(() => { + fakePersonID = faker.string.uuid(); + fakeDN = faker.string.alpha(); + }); + + describe('when', () => { + it('should publish event and return new (UEM) userPassword', async () => { + ldapClientMock.getClient.mockImplementation(() => { + clientMock.bind.mockResolvedValueOnce(); + clientMock.search.mockResolvedValueOnce( + createMock({ + searchEntries: [ + createMock({ + dn: fakeDN, + }), + ], + }), + ); + clientMock.modify.mockResolvedValueOnce(undefined); + + return clientMock; + }); + + const result: Result = await ldapClientService.changeUserPasswordByPersonId( + fakePersonID, + faker.internet.userName(), + ); + + if (!result.ok) throw Error(); + expect(result.value).toHaveLength(8); + expect(loggerMock.info).toHaveBeenLastCalledWith( + `LDAP: Successfully modified userPassword (UEM) for personId:${fakePersonID}`, + ); + expect(eventServiceMock.publish).toHaveBeenCalledTimes(1); + }); + }); + }); + }); }); diff --git a/src/core/ldap/domain/ldap-client.service.ts b/src/core/ldap/domain/ldap-client.service.ts index 3f52e81f5..02a97318c 100644 --- a/src/core/ldap/domain/ldap-client.service.ts +++ b/src/core/ldap/domain/ldap-client.service.ts @@ -7,15 +7,15 @@ import { LdapInstanceConfig } from '../ldap-instance-config.js'; import { UsernameRequiredError } from '../../../modules/person/domain/username-required.error.js'; import { Mutex } from 'async-mutex'; import { LdapSearchError } from '../error/ldap-search.error.js'; -import { PersonID } from '../../../shared/types/aggregate-ids.types.js'; +import { PersonID, PersonReferrer } from '../../../shared/types/aggregate-ids.types.js'; 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'; -import { PersonRepository } from '../../../modules/person/persistence/person.repository.js'; -import { Person } from '../../../modules/person/domain/person.js'; +import { LdapModifyUserPasswordError } from '../error/ldap-modify-user-password.error.js'; +import { generatePassword } from '../../../shared/util/password-generator.js'; export type PersonData = { vorname: string; @@ -39,6 +39,8 @@ export class LdapClientService { public static readonly MAIL_ALTERNATIVE_ADDRESS: string = 'mailAlternativeAddress'; + public static readonly USER_PASSWORD: string = 'userPassword'; + 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 @@ -56,7 +58,6 @@ export class LdapClientService { private readonly ldapInstanceConfig: LdapInstanceConfig, private readonly logger: ClassLogger, private readonly eventService: EventService, - private readonly personRepo: PersonRepository, ) { this.mutex = new Mutex(); } @@ -115,12 +116,6 @@ export class LdapClientService { return rootName; } - private async getPersonReferrerOrUndefined(personId: PersonID): Promise { - const person: Option> = await this.personRepo.findById(personId); - - return person?.referrer; - } - public async createLehrer( person: PersonData, domain: string, @@ -340,18 +335,13 @@ export class LdapClientService { }); } - public async changeEmailAddressByPersonId(personId: PersonID, newEmailAddress: string): Promise> { - const referrer: string | undefined = await this.getPersonReferrerOrUndefined(personId); - if (!referrer) { - this.logger.error( - `Changing email-address in LDAP FAILED, no person/referrer found for personId:${personId}`, - ); - return { - ok: false, - error: new LdapSearchError(LdapEntityType.LEHRER), - }; - } - + public async changeEmailAddressByPersonId( + personId: PersonID, + referrer: PersonReferrer, + newEmailAddress: string, + ): Promise> { + // Converted to avoid PersonRepository-ref, UEM-password-generation + //const referrer: string | undefined = await this.getPersonReferrerOrUndefined(personId); return this.mutex.runExclusive(async () => { this.logger.info('LDAP: changeEmailAddress'); const splitted: string[] = newEmailAddress.split('@'); @@ -434,4 +424,52 @@ export class LdapClientService { } }); } + + public async changeUserPasswordByPersonId(personId: PersonID, referrer: PersonReferrer): Promise> { + // Converted to avoid PersonRepository-ref, UEM-password-generation + //const referrer: string | undefined = await this.getPersonReferrerOrUndefined(personId); + const userPassword: string = generatePassword(); + + return this.mutex.runExclusive(async () => { + this.logger.info('LDAP: changeUserPassword'); + const client: Client = this.ldapClient.getClient(); + const bindResult: Result = await this.bind(); + if (!bindResult.ok) return bindResult; + + const searchResult: SearchResult = await client.search(`${LdapClientService.DC_SCHULE_SH_DC_DE}`, { + scope: 'sub', + filter: `(uid=${referrer})`, + attributes: [LdapClientService.USER_PASSWORD], + returnAttributeValues: true, + }); + if (!searchResult.searchEntries[0]) { + this.logger.error(`Modification FAILED, no entry for person:${referrer}`); + return { + ok: false, + error: new LdapSearchError(LdapEntityType.LEHRER), + }; + } + + try { + await client.modify(searchResult.searchEntries[0].dn, [ + new Change({ + operation: 'replace', + modification: new Attribute({ + type: LdapClientService.USER_PASSWORD, + values: [userPassword], + }), + }), + ]); + this.logger.info(`LDAP: Successfully modified userPassword (UEM) for personId:${personId}`); + this.eventService.publish(new LdapPersonEntryChangedEvent(personId, undefined, undefined, true)); + + return { ok: true, value: userPassword }; + } catch (err) { + const errMsg: string = JSON.stringify(err); + this.logger.error(`LDAP: Modifying userPassword (UEM) FAILED, errMsg:${errMsg}`); + + return { ok: false, error: new LdapModifyUserPasswordError() }; + } + }); + } } diff --git a/src/core/ldap/domain/ldap-event-handler.spec.ts b/src/core/ldap/domain/ldap-event-handler.spec.ts index f085b292f..c57aec660 100644 --- a/src/core/ldap/domain/ldap-event-handler.spec.ts +++ b/src/core/ldap/domain/ldap-event-handler.spec.ts @@ -511,6 +511,7 @@ describe('LDAP Event Handler', () => { it('should call ldap client changeEmailAddressByPersonId', async () => { const event: EmailAddressGeneratedEvent = new EmailAddressGeneratedEvent( faker.string.uuid(), + faker.internet.userName(), faker.string.uuid(), faker.internet.email(), true, @@ -530,6 +531,7 @@ describe('LDAP Event Handler', () => { it('should call ldap client changeEmailAddressByPersonId', async () => { const event: EmailAddressChangedEvent = new EmailAddressChangedEvent( faker.string.uuid(), + faker.internet.userName(), faker.string.uuid(), faker.internet.email(), faker.string.uuid(), diff --git a/src/core/ldap/domain/ldap-event-handler.ts b/src/core/ldap/domain/ldap-event-handler.ts index a03d8250a..e19b0c13b 100644 --- a/src/core/ldap/domain/ldap-event-handler.ts +++ b/src/core/ldap/domain/ldap-event-handler.ts @@ -183,7 +183,7 @@ export class LdapEventHandler { `Received EmailAddressGeneratedEvent, personId:${event.personId}, emailAddress: ${event.address}`, ); - await this.ldapClientService.changeEmailAddressByPersonId(event.personId, event.address); + await this.ldapClientService.changeEmailAddressByPersonId(event.personId, event.referrer, event.address); } @EventHandler(EmailAddressChangedEvent) @@ -192,6 +192,6 @@ export class LdapEventHandler { `Received EmailAddressChangedEvent, personId:${event.personId}, newEmailAddress: ${event.newAddress}`, ); - await this.ldapClientService.changeEmailAddressByPersonId(event.personId, event.newAddress); + await this.ldapClientService.changeEmailAddressByPersonId(event.personId, event.referrer, event.newAddress); } } diff --git a/src/core/ldap/error/ldap-modify-user-password.error.ts b/src/core/ldap/error/ldap-modify-user-password.error.ts new file mode 100644 index 000000000..772c76129 --- /dev/null +++ b/src/core/ldap/error/ldap-modify-user-password.error.ts @@ -0,0 +1,7 @@ +import { DomainError } from '../../../shared/error/index.js'; + +export class LdapModifyUserPasswordError extends DomainError { + public constructor(details?: unknown[] | Record) { + super(`LDAP error: Modifying userPassword FAILED`, 'LDAP_USER_PASSWORD_MODIFICATION_FAILED', details); + } +} diff --git a/src/core/ldap/ldap.module.ts b/src/core/ldap/ldap.module.ts index bf917bdf7..d989515e3 100644 --- a/src/core/ldap/ldap.module.ts +++ b/src/core/ldap/ldap.module.ts @@ -4,20 +4,11 @@ import { LdapEventHandler } from './domain/ldap-event-handler.js'; import { LdapClientService } from './domain/ldap-client.service.js'; import { RolleModule } from '../../modules/rolle/rolle.module.js'; import { OrganisationModule } from '../../modules/organisation/organisation.module.js'; -import { PersonModule } from '../../modules/person/person.module.js'; import { LdapConfigModule } from './ldap-config.module.js'; -import { PersonenKontextModule } from '../../modules/personenkontext/personenkontext.module.js'; import { LdapClient } from './domain/ldap-client.js'; @Module({ - imports: [ - LoggerModule.register(LdapModule.name), - LdapConfigModule, - RolleModule, - PersonModule, - OrganisationModule, - PersonenKontextModule, - ], + imports: [LoggerModule.register(LdapModule.name), LdapConfigModule, RolleModule, OrganisationModule], providers: [LdapEventHandler, LdapClientService, LdapClient], exports: [LdapEventHandler, LdapClientService, LdapClient], }) diff --git a/src/modules/email/domain/email-event-handler.spec.ts b/src/modules/email/domain/email-event-handler.spec.ts index 738b75acb..bf85cc012 100644 --- a/src/modules/email/domain/email-event-handler.spec.ts +++ b/src/modules/email/domain/email-event-handler.spec.ts @@ -148,7 +148,7 @@ describe('EmailEventHandler', () => { }); } - describe('test private methods: createOrEnableEmail, createNewEmail, changeEmail', () => { + describe('test private methods: createOrEnableEmail, createNewEmail, changeEmail, getPersonReferrerOrError', () => { let fakePersonId: PersonID; let fakeRolleId: RolleID; let fakeOrgaId: string; @@ -288,6 +288,27 @@ describe('EmailEventHandler', () => { }); }); }); + + describe('getPersonReferrerOrError', () => { + describe('when personReferrer is NOT defined', () => { + it('should log matching error', async () => { + dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); + rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); + serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(undefined); //no existing email is found + organisationRepositoryMock.findById.mockResolvedValue(createMock>()); + //mock person without referrer is found + personRepositoryMock.findById.mockResolvedValueOnce( + createMock>({ id: fakePersonId, referrer: undefined }), + ); + await emailEventHandler.handlePersonRenamedEvent(personRenamedEvent); + + expect(loggerMock.error).toHaveBeenCalledWith( + `Referrer Could Not Be Found For personId:${fakePersonId}`, + ); + }); + }); + }); }); describe('handlePersonenkontextUpdatedEvent', () => { @@ -321,7 +342,7 @@ describe('EmailEventHandler', () => { organisationRepositoryMock.findById.mockResolvedValue(createMock>()); }); - describe('when email exists and is enabled', () => { + describe('when email exists, person with referrer can be found and is enabled', () => { it('should log matching info', async () => { dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); @@ -339,6 +360,9 @@ describe('EmailEventHandler', () => { ), ]); + //mock person with referrer is found + personRepositoryMock.findById.mockResolvedValueOnce(createMock>()); + await emailEventHandler.handlePersonenkontextUpdatedEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( @@ -347,12 +371,50 @@ describe('EmailEventHandler', () => { }); }); + //test case to cover case: getPersonReferrerOrError is returning error + describe('when email exists, person WITHOUT referrer is found and is enabled', () => { + it('should log matching info', async () => { + dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); + rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); + serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + + // eslint-disable-next-line @typescript-eslint/require-await + emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockImplementationOnce(async (personId: PersonID) => [ + new EmailAddress( + faker.string.uuid(), + faker.date.past(), + faker.date.recent(), + personId, + faker.internet.email(), + EmailAddressStatus.ENABLED, + ), + ]); + + //mock person with referrer is found + personRepositoryMock.findById.mockResolvedValueOnce(createMock>({ referrer: undefined })); + + await emailEventHandler.handlePersonenkontextUpdatedEvent(event); + + expect(loggerMock.error).toHaveBeenCalledWith( + `Referrer Could Not Be Found For personId:${fakePersonId}`, + ); + expect(loggerMock.info).not.toHaveBeenCalledWith( + `Existing email for personId:${fakePersonId} already enabled`, + ); + }); + }); + describe('when email exists but is disabled and enabling is successful', () => { it('should log matching info', async () => { dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + //mock person with referrer is found + personRepositoryMock.findById.mockResolvedValueOnce( + createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + ); + // eslint-disable-next-line @typescript-eslint/require-await emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockImplementationOnce(async (personId: PersonID) => [ new EmailAddress( @@ -382,6 +444,11 @@ describe('EmailEventHandler', () => { rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); + //mock person with referrer is found + personRepositoryMock.findById.mockResolvedValueOnce( + createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + ); + // eslint-disable-next-line @typescript-eslint/require-await emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockImplementationOnce(async (personId: PersonID) => [ new EmailAddress( @@ -409,8 +476,11 @@ describe('EmailEventHandler', () => { dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); - emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found + //mock person with referrer is found + personRepositoryMock.findById.mockResolvedValue( + createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + ); const persistenceResult: EmailAddress = getEmail(); emailRepoMock.save.mockResolvedValueOnce(persistenceResult); //mock: error during saving the entity @@ -444,8 +514,11 @@ describe('EmailEventHandler', () => { dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); - emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found + //mock person with referrer is found + personRepositoryMock.findById.mockResolvedValue( + createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + ); // eslint-disable-next-line @typescript-eslint/require-await emailFactoryMock.createNew.mockImplementationOnce(async (personId: PersonID) => { @@ -482,8 +555,11 @@ describe('EmailEventHandler', () => { dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); - emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found + //mock person with referrer is found + personRepositoryMock.findById.mockResolvedValue( + createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + ); // eslint-disable-next-line @typescript-eslint/require-await emailFactoryMock.createNew.mockImplementationOnce(async (personId: PersonID) => { @@ -510,8 +586,11 @@ describe('EmailEventHandler', () => { dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); - emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found + //mock person with referrer is found + personRepositoryMock.findById.mockResolvedValue( + createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + ); emailRepoMock.save.mockResolvedValueOnce(new EmailAddressNotFoundError(fakeEmailAddressString)); //mock: error during saving the entity @@ -827,6 +906,10 @@ describe('EmailEventHandler', () => { rolleRepoMock.findByIds.mockResolvedValueOnce(rollenMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(emailAddress); + //mock person with referrer is found + personRepositoryMock.findById.mockResolvedValueOnce( + createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + ); emailRepoMock.save.mockResolvedValueOnce(emailAddress); @@ -851,6 +934,10 @@ describe('EmailEventHandler', () => { rolleRepoMock.findByIds.mockResolvedValueOnce(rollenMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(emailAddress); + //mock person with referrer is found + personRepositoryMock.findById.mockResolvedValueOnce( + createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + ); emailRepoMock.save.mockResolvedValueOnce(emailAddress); @@ -881,6 +968,10 @@ describe('EmailEventHandler', () => { rolleRepoMock.findByIds.mockResolvedValueOnce(rollenMap); serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap); emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(emailAddress); + //mock person with referrer is found + personRepositoryMock.findById.mockResolvedValueOnce( + createMock>({ id: faker.string.uuid(), referrer: faker.internet.userName() }), + ); emailRepoMock.save.mockResolvedValueOnce(emailAddress); diff --git a/src/modules/email/domain/email-event-handler.ts b/src/modules/email/domain/email-event-handler.ts index a572c6160..210d036f3 100644 --- a/src/modules/email/domain/email-event-handler.ts +++ b/src/modules/email/domain/email-event-handler.ts @@ -32,6 +32,7 @@ import { EmailAddressAlreadyExistsEvent } from '../../../shared/events/email-add import { EmailAddressDisabledEvent } from '../../../shared/events/email-address-disabled.event.js'; import { PersonRepository } from '../../person/persistence/person.repository.js'; import { Person } from '../../person/domain/person.js'; +import { PersonDomainError } from '../../person/domain/person-domain.error.js'; type RolleWithPK = { rolle: Rolle; @@ -276,6 +277,31 @@ export class EmailEventHandler { } } + private async getPersonReferrerOrError(personId: PersonID): Promise> { + const person: Option> = await this.personRepository.findById(personId); + + if (!person) { + this.logger.error(`Person Could Not Be Found For personId:${personId}`); + return { + ok: false, + error: new EntityNotFoundError('Person', personId), + }; + } + if (!person.referrer) { + this.logger.error(`Referrer Could Not Be Found For personId:${personId}`); + return { + ok: false, + error: new PersonDomainError('Person-Referrer NOT defined', personId), + }; + } + + this.logger.info(`Found referrer${person.referrer} For personId:${personId}`); + return { + ok: true, + value: person.referrer, + }; + } + private async handlePerson(personId: PersonID): Promise { // Map to store combinations of rolleId and organisationId as the key const rolleIdPKMap: Map> = new Map>(); @@ -397,6 +423,10 @@ export class EmailEventHandler { this.eventService.publish(new EmailAddressAlreadyExistsEvent(personId, organisationKennung.value)); } + const personReferrer: Result = await this.getPersonReferrerOrError(personId); + if (!personReferrer.ok) { + return; //error logging is done in getPersonReferrerOrError + } for (const email of existingEmails) { if (email.enabled) { return this.logger.info(`Existing email for personId:${personId} already enabled`); @@ -412,6 +442,7 @@ export class EmailEventHandler { this.eventService.publish( new EmailAddressGeneratedEvent( personId, + personReferrer.value, persistenceResult.id, persistenceResult.address, persistenceResult.enabled, @@ -450,6 +481,10 @@ export class EmailEventHandler { private async createNewEmail(personId: PersonID, organisationId: OrganisationID): Promise { const organisationKennung: Result = await this.getOrganisationKennung(organisationId); if (!organisationKennung.ok) return; + const personReferrer: Result = await this.getPersonReferrerOrError(personId); + if (!personReferrer.ok) { + return; //error logging is done in getPersonReferrerOrError + } const email: Result> = await this.emailFactory.createNew(personId, organisationId); if (!email.ok) { await this.createAndPersistFailedEmailAddress(personId); @@ -464,6 +499,7 @@ export class EmailEventHandler { this.eventService.publish( new EmailAddressGeneratedEvent( personId, + personReferrer.value, persistenceResult.id, persistenceResult.address, persistenceResult.enabled, @@ -482,6 +518,10 @@ export class EmailEventHandler { ): Promise { const organisationKennung: Result = await this.getOrganisationKennung(organisationId); if (!organisationKennung.ok) return; + const personReferrer: Result = await this.getPersonReferrerOrError(personId); + if (!personReferrer.ok) { + return; //error logging is done in getPersonReferrerOrError + } const email: Result> = await this.emailFactory.createNew(personId, organisationId); if (!email.ok) { await this.createAndPersistFailedEmailAddress(personId); @@ -497,6 +537,7 @@ export class EmailEventHandler { this.eventService.publish( new EmailAddressChangedEvent( personId, + personReferrer.value, oldEmail.id, oldEmail.address, persistenceResult.id, diff --git a/src/modules/ox/domain/ox-event-handler.spec.ts b/src/modules/ox/domain/ox-event-handler.spec.ts index 661c119b2..db3492064 100644 --- a/src/modules/ox/domain/ox-event-handler.spec.ts +++ b/src/modules/ox/domain/ox-event-handler.spec.ts @@ -129,6 +129,7 @@ describe('OxEventHandler', () => { fakeDstNr = faker.string.numeric(); event = new EmailAddressGeneratedEvent( personId, + faker.internet.userName(), faker.string.uuid(), faker.internet.email(), true, @@ -190,6 +191,7 @@ describe('OxEventHandler', () => { fakeDstNr = faker.string.numeric(); event = new EmailAddressGeneratedEvent( personId, + faker.internet.userName(), faker.string.uuid(), faker.internet.email(), true, @@ -414,6 +416,7 @@ describe('OxEventHandler', () => { fakeDstNr = faker.string.numeric(); event = new EmailAddressGeneratedEvent( personId, + faker.internet.userName(), faker.string.uuid(), faker.internet.email(), true, @@ -481,6 +484,7 @@ describe('OxEventHandler', () => { personId = faker.string.uuid(); event = new EmailAddressGeneratedEvent( personId, + faker.internet.userName(), faker.string.uuid(), faker.internet.email(), true, @@ -772,6 +776,7 @@ describe('OxEventHandler', () => { contextName: faker.string.alpha(); event = new EmailAddressChangedEvent( personId, + faker.internet.userName(), faker.string.uuid(), faker.internet.email(), faker.string.uuid(), diff --git a/src/modules/person/api/dbiam-person.error.ts b/src/modules/person/api/dbiam-person.error.ts index 45da7c3a7..4ec8ce27e 100644 --- a/src/modules/person/api/dbiam-person.error.ts +++ b/src/modules/person/api/dbiam-person.error.ts @@ -10,6 +10,7 @@ export enum PersonErrorI18nTypes { PERSONALNUMMER_REQUIRED = 'PERSONALNUMMER_REQUIRED', NEWER_VERSION_OF_PERSON_AVAILABLE = 'NEWER_VERSION_OF_PERSON_AVAILABLE', PERSONALNUMMER_NICHT_EINDEUTIG = 'PERSONALNUMMER_NICHT_EINDEUTIG', + PERSON_UEM_PASSWORD_MODIFICATION_ERROR = 'PERSON_UEM_PASSWORD_MODIFICATION_ERROR', } export type DbiamPersonErrorProps = DbiamErrorProps & { diff --git a/src/modules/person/api/person-exception-filter.ts b/src/modules/person/api/person-exception-filter.ts index 1616857ba..737a7f56a 100644 --- a/src/modules/person/api/person-exception-filter.ts +++ b/src/modules/person/api/person-exception-filter.ts @@ -10,6 +10,7 @@ import { DownstreamKeycloakError } from '../domain/person-keycloak.error.js'; import { PersonUpdateOutdatedError } from '../domain/update-outdated.error.js'; import { DuplicatePersonalnummerError } from '../../../shared/error/duplicate-personalnummer.error.js'; import { PersonalnummerRequiredError } from '../domain/personalnummer-required.error.js'; +import { PersonUserPasswordModificationError } from '../domain/person-user-password-modification.error.js'; @Catch(PersonDomainError, DuplicatePersonalnummerError) export class PersonExceptionFilter implements ExceptionFilter { @@ -63,6 +64,13 @@ export class PersonExceptionFilter implements ExceptionFilter i18nKey: PersonErrorI18nTypes.PERSONALNUMMER_NICHT_EINDEUTIG, }), ], + [ + PersonUserPasswordModificationError.name, + new DbiamPersonError({ + code: 500, + i18nKey: PersonErrorI18nTypes.PERSON_UEM_PASSWORD_MODIFICATION_ERROR, + }), + ], ]); public catch(exception: PersonDomainError, host: ArgumentsHost): void { diff --git a/src/modules/person/api/person.controller.spec.ts b/src/modules/person/api/person.controller.spec.ts index 9b52e8433..d29f1eba6 100644 --- a/src/modules/person/api/person.controller.spec.ts +++ b/src/modules/person/api/person.controller.spec.ts @@ -44,6 +44,8 @@ import { EmailRepo } from '../../email/persistence/email.repo.js'; import { PersonEmailResponse } from './person-email-response.js'; import { EmailAddressStatus } from '../../email/domain/email-address.js'; import { PersonLockOccasion } from '../domain/person.enums.js'; +import { LdapClientService } from '../../../core/ldap/domain/ldap-client.service.js'; +import { PersonUserPasswordModificationError } from '../domain/person-user-password-modification.error.js'; describe('PersonController', () => { let module: TestingModule; @@ -58,6 +60,7 @@ describe('PersonController', () => { let personPermissionsMock: DeepMocked; let dBiamPersonenkontextServiceMock: DeepMocked; let eventServiceMock: DeepMocked; + let ldapClientServiceMock: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -118,6 +121,10 @@ describe('PersonController', () => { provide: EmailRepo, useValue: createMock(), }, + { + provide: LdapClientService, + useValue: createMock(), + }, ], }).compile(); personController = module.get(PersonController); @@ -130,6 +137,7 @@ describe('PersonController', () => { keycloakUserService = module.get(KeycloakUserService); dBiamPersonenkontextServiceMock = module.get(DBiamPersonenkontextService); eventServiceMock = module.get(EventService); + ldapClientServiceMock = module.get(LdapClientService); }); function getPerson(): Person { @@ -878,8 +886,8 @@ describe('PersonController', () => { personId: faker.string.uuid(), }; const body: PersonMetadataBodyParams = { - familienname: faker.name.lastName(), - vorname: faker.name.firstName(), + familienname: faker.person.lastName(), + vorname: faker.person.firstName(), personalnummer: faker.finance.pin(7), lastModified: faker.date.recent(), revision: '1', @@ -930,8 +938,8 @@ describe('PersonController', () => { it('should throw HttpException when revision is incorrect', async () => { const bodyWithInvalidRevision: PersonMetadataBodyParams = { - familienname: faker.name.lastName(), - vorname: faker.name.firstName(), + familienname: faker.person.lastName(), + vorname: faker.person.firstName(), personalnummer: '', lastModified: faker.date.recent(), revision: '2', @@ -940,6 +948,7 @@ describe('PersonController', () => { true, ); personRepositoryMock.updatePersonMetadata.mockResolvedValue(new MismatchedRevisionError('')); + await expect( personController.updateMetadata(params, bodyWithInvalidRevision, personPermissionsMock), ).rejects.toThrow(HttpException); @@ -954,4 +963,120 @@ describe('PersonController', () => { ); }); }); + + describe('resetUEMPasswordByPersonId', () => { + describe('when person does not exist', () => { + const params: PersonByIdParams = { + personId: faker.string.uuid(), + }; + personPermissionsMock = createMock(); + + it('should throw HttpException', async () => { + personRepositoryMock.findBy.mockResolvedValue([[], 0]); + personRepositoryMock.getPersonIfAllowedOrRequesterIsPerson.mockResolvedValueOnce({ + ok: false, + error: new EntityNotFoundError(), + }); + + await expect( + personController.resetUEMPasswordByPersonId(params, personPermissionsMock), + ).rejects.toThrow(HttpException); + expect(personRepositoryMock.update).toHaveBeenCalledTimes(0); + }); + }); + + describe('when permissions are insufficient to reset user-password', () => { + const params: PersonByIdParams = { + personId: faker.string.uuid(), + }; + personPermissionsMock = createMock(); + + it('should throw HttpNotFoundException', async () => { + personRepositoryMock.findById.mockResolvedValue(undefined); + personRepositoryMock.getPersonIfAllowedOrRequesterIsPerson.mockResolvedValueOnce({ + ok: false, + error: new EntityNotFoundError(), + }); + + await expect( + personController.resetUEMPasswordByPersonId(params, personPermissionsMock), + ).rejects.toThrow(HttpException); + expect(personRepositoryMock.update).toHaveBeenCalledTimes(0); + }); + }); + + describe('when person does NOT have a defined referrer', () => { + const params: PersonByIdParams = { + personId: faker.string.uuid(), + }; + personPermissionsMock = createMock(); + + it('should throw HttpException', async () => { + personRepositoryMock.findBy.mockResolvedValue([[], 0]); + personRepositoryMock.getPersonIfAllowedOrRequesterIsPerson.mockResolvedValueOnce({ + ok: true, + value: createMock>({ referrer: undefined }), + }); + + await expect( + personController.resetUEMPasswordByPersonId(params, personPermissionsMock), + ).rejects.toThrow(HttpException); + expect(personRepositoryMock.update).toHaveBeenCalledTimes(0); + }); + }); + + describe('when resetting UEM-password for a person by personId succeeds', () => { + const params: PersonByIdParams = { + personId: faker.string.uuid(), + }; + const person: Person = getPerson(); + personPermissionsMock = createMock(); + + it('should reset UEM-password for person', async () => { + personRepositoryMock.findById.mockResolvedValue(person); + personRepositoryMock.getPersonIfAllowedOrRequesterIsPerson.mockResolvedValueOnce({ + ok: true, + value: person, + }); + + await expect( + personController.resetUEMPasswordByPersonId(params, personPermissionsMock), + ).resolves.not.toThrow(); + expect(ldapClientServiceMock.changeUserPasswordByPersonId).toHaveBeenCalledTimes(1); + expect(ldapClientServiceMock.changeUserPasswordByPersonId).toHaveBeenCalledWith( + person.id, + person.referrer, + ); + }); + }); + + describe('when resetting UEM-password for a person returns a SchulConnexError', () => { + const params: PersonByIdParams = { + personId: faker.string.uuid(), + }; + const person: Person = getPerson(); + personPermissionsMock = createMock(); + + it('should throw HttpException', async () => { + personRepositoryMock.findById.mockResolvedValue(person); + ldapClientServiceMock.changeUserPasswordByPersonId.mockResolvedValueOnce({ + ok: false, + error: new PersonDomainError('Person', 'entityId', undefined), + }); + personRepositoryMock.getPersonIfAllowedOrRequesterIsPerson.mockResolvedValueOnce({ + ok: true, + value: person, + }); + + await expect( + personController.resetUEMPasswordByPersonId(params, personPermissionsMock), + ).rejects.toThrow(PersonUserPasswordModificationError); + expect(ldapClientServiceMock.changeUserPasswordByPersonId).toHaveBeenCalledTimes(1); + expect(ldapClientServiceMock.changeUserPasswordByPersonId).toHaveBeenCalledWith( + person.id, + person.referrer, + ); + }); + }); + }); }); diff --git a/src/modules/person/api/person.controller.ts b/src/modules/person/api/person.controller.ts index 6df2fd834..ebc3ba562 100644 --- a/src/modules/person/api/person.controller.ts +++ b/src/modules/person/api/person.controller.ts @@ -78,6 +78,9 @@ import { PersonEmailResponse } from './person-email-response.js'; import { UserLock } from '../../keycloak-administration/domain/user-lock.js'; import { StepUpGuard } from '../../authentication/api/steup-up.guard.js'; import { PersonLockOccasion } from '../domain/person.enums.js'; +import { LdapClientService } from '../../../core/ldap/domain/ldap-client.service.js'; +import { PersonID } from '../../../shared/types/aggregate-ids.types.js'; +import { PersonUserPasswordModificationError } from '../domain/person-user-password-modification.error.js'; @UseFilters(SchulConnexValidationErrorFilter, new AuthenticationExceptionFilter(), new PersonExceptionFilter()) @ApiTags('personen') @@ -96,9 +99,10 @@ export class PersonController { private readonly logger: ClassLogger, private keycloakUserService: KeycloakUserService, private readonly dBiamPersonenkontextService: DBiamPersonenkontextService, - config: ConfigService, + private readonly ldapClientService: LdapClientService, private readonly personApiMapper: PersonApiMapper, private readonly eventService: EventService, + config: ConfigService, ) { this.ROOT_ORGANISATION_ID = config.getOrThrow('DATA').ROOT_ORGANISATION_ID; } @@ -537,4 +541,45 @@ export class PersonController { } return new PersonendatensatzResponse(result, false); } + + @Patch(':personId/uem-password') + @HttpCode(HttpStatus.ACCEPTED) + @ApiAcceptedResponse({ description: 'UEM-password for person was successfully reset.', type: String }) + @ApiNotFoundResponse({ description: 'The person does not exist or insufficient permissions to update person.' }) + @ApiInternalServerErrorResponse({ description: 'Internal server error.' }) + @UseInterceptors(ResultInterceptor) + public async resetUEMPasswordByPersonId( + @Param() params: PersonByIdParams, + @Permissions() permissions: PersonPermissions, + ): Promise> { + //check that logged-in user is allowed to update person + const personResult: Result> = await this.personRepository.getPersonIfAllowedOrRequesterIsPerson( + params.personId, + permissions, + ); + if (!personResult.ok) { + throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( + SchulConnexErrorMapper.mapDomainErrorToSchulConnexError( + new EntityNotFoundError('Person', params.personId), + ), + ); + } + if (!personResult.value.referrer) { + throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException( + SchulConnexErrorMapper.mapDomainErrorToSchulConnexError( + new PersonDomainError('Person-Referrer NOT defined', params.personId), + ), + ); + } + const changeUserPasswordResult: Result = await this.ldapClientService.changeUserPasswordByPersonId( + personResult.value.id, + personResult.value.referrer, + ); + + if (!changeUserPasswordResult.ok) { + throw new PersonUserPasswordModificationError(personResult.value.id); + } + + return { ok: true, value: changeUserPasswordResult.value }; + } } diff --git a/src/modules/person/api/personenuebersicht/dbiam-personenzuordnung.response.ts b/src/modules/person/api/personenuebersicht/dbiam-personenzuordnung.response.ts index 66d30d367..55f061ab5 100644 --- a/src/modules/person/api/personenuebersicht/dbiam-personenzuordnung.response.ts +++ b/src/modules/person/api/personenuebersicht/dbiam-personenzuordnung.response.ts @@ -4,7 +4,7 @@ import { Personenkontext } from '../../../personenkontext/domain/personenkontext import { Rolle } from '../../../rolle/domain/rolle.js'; import { OrganisationDo } from '../../../organisation/domain/organisation.do.js'; import { OrganisationsTyp, OrganisationsTypName } from '../../../organisation/domain/organisation.enums.js'; -import { RollenMerkmal, RollenMerkmalTypName } from '../../../rolle/domain/rolle.enums.js'; +import { RollenArt, RollenArtTypName, RollenMerkmal, RollenMerkmalTypName } from '../../../rolle/domain/rolle.enums.js'; export class DBiamPersonenzuordnungResponse { @ApiProperty({ type: String }) @@ -22,6 +22,9 @@ export class DBiamPersonenzuordnungResponse { @ApiProperty({ type: String }) public readonly rolle: string; + @ApiProperty({ enum: RollenArt, enumName: RollenArtTypName, nullable: false }) + public readonly rollenArt: RollenArt; + @ApiProperty({ type: String }) public readonly administriertVon?: string; @@ -49,6 +52,7 @@ export class DBiamPersonenzuordnungResponse { this.sskName = organisation.name!; this.sskDstNr = organisation.kennung!; this.rolle = rolle.name; + this.rollenArt = rolle.rollenart; this.administriertVon = organisation.administriertVon; this.typ = organisation.typ; this.editable = editable; diff --git a/src/modules/person/domain/person-user-password-modification.error.ts b/src/modules/person/domain/person-user-password-modification.error.ts new file mode 100644 index 000000000..26e689bfb --- /dev/null +++ b/src/modules/person/domain/person-user-password-modification.error.ts @@ -0,0 +1,7 @@ +import { PersonDomainError } from './person-domain.error.js'; + +export class PersonUserPasswordModificationError extends PersonDomainError { + public constructor(entityId: string | undefined, details?: unknown[] | Record) { + super(`Attribute 'userPassword' (UEM) could not be modified for Person with id ${entityId}`, entityId, details); + } +} diff --git a/src/modules/person/persistence/person.repository.integration-spec.ts b/src/modules/person/persistence/person.repository.integration-spec.ts index 813d49d2d..f17e42ba2 100644 --- a/src/modules/person/persistence/person.repository.integration-spec.ts +++ b/src/modules/person/persistence/person.repository.integration-spec.ts @@ -1324,6 +1324,122 @@ describe('PersonRepository Integration', () => { } }); }); + + describe('getPersonIfAllowedOrRequesterIsPerson', () => { + describe('when requester id equals personId and can be found', () => { + it('should return person and not call getPersonIfAllowed', async () => { + const person1: Person = DoFactory.createPerson(true); + const personEntity: PersonEntity = new PersonEntity(); + await em.persistAndFlush(personEntity.assign(mapAggregateToData(person1))); + person1.id = personEntity.id; + + //mock requester is same person + personPermissionsMock.personFields.id = person1.id; + + kcUserServiceMock.findById.mockResolvedValue({ + ok: true, + value: { + id: person1.keycloakUserId!, + username: person1.username ?? '', + enabled: true, + email: faker.internet.email(), + createdDate: new Date(), + externalSystemIDs: {}, + attributes: {}, + }, + }); + + const result: Result> = await sut.getPersonIfAllowedOrRequesterIsPerson( + person1.id, + personPermissionsMock, + ); + + expect(result.ok).toBeTruthy(); + expect(personPermissionsMock.getOrgIdsWithSystemrecht).toHaveBeenCalledTimes(0); + }); + }); + + describe('when requester id equals personId and CANNOT be found', () => { + it('should return person and not call getPersonIfAllowed', async () => { + const nonExistingPersonId: string = faker.string.uuid(); + //mock requester is same person + personPermissionsMock.personFields.id = nonExistingPersonId; + + const result: Result> = await sut.getPersonIfAllowedOrRequesterIsPerson( + nonExistingPersonId, + personPermissionsMock, + ); + + expect(result.ok).toBeFalsy(); + expect(personPermissionsMock.getOrgIdsWithSystemrecht).toHaveBeenCalledTimes(0); + }); + }); + + describe('when requester id is NOT equal personId', () => { + it('should call getPersonIfAllowed', async () => { + const person1: Person = DoFactory.createPerson(true); + const personEntity: PersonEntity = new PersonEntity(); + await em.persistAndFlush(personEntity.assign(mapAggregateToData(person1))); + person1.id = personEntity.id; + + const organisation: OrganisationEntity = await createAndPersistOrganisation( + em, + undefined, + OrganisationsTyp.SCHULE, + ); + + const rolleData: RequiredEntityData = { + name: 'Testrolle', + administeredBySchulstrukturknoten: organisation.id, + rollenart: RollenArt.ORGADMIN, + istTechnisch: false, + }; + const rolleEntity: RolleEntity = em.create(RolleEntity, rolleData); + await em.persistAndFlush(rolleEntity); + + const personenkontextData: RequiredEntityData = { + organisationId: organisation.id, + personId: person1.id, + rolleId: rolleEntity.id, + }; + const personenkontextEntity: PersonenkontextEntity = em.create( + PersonenkontextEntity, + personenkontextData, + ); + await em.persistAndFlush(personenkontextEntity); + + //mock explicitly that requestor is another person + personPermissionsMock.personFields.id = faker.string.uuid(); + + personPermissionsMock.getOrgIdsWithSystemrecht.mockResolvedValue({ + all: false, + orgaIds: [organisation.id], + }); + + kcUserServiceMock.findById.mockResolvedValue({ + ok: true, + value: { + id: person1.keycloakUserId!, + username: person1.username ?? '', + enabled: true, + email: faker.internet.email(), + createdDate: new Date(), + externalSystemIDs: {}, + attributes: {}, + }, + }); + + const result: Result> = await sut.getPersonIfAllowedOrRequesterIsPerson( + person1.id, + personPermissionsMock, + ); + + expect(result.ok).toBeTruthy(); + expect(personPermissionsMock.getOrgIdsWithSystemrecht).toHaveBeenCalledTimes(1); + }); + }); + }); + describe('deletePerson', () => { describe('Delete the person and all kontexte', () => { afterEach(() => { diff --git a/src/modules/person/persistence/person.repository.ts b/src/modules/person/persistence/person.repository.ts index f0592690d..698e94490 100644 --- a/src/modules/person/persistence/person.repository.ts +++ b/src/modules/person/persistence/person.repository.ts @@ -246,6 +246,20 @@ export class PersonRepository { return { ok: true, value: person }; } + public async getPersonIfAllowedOrRequesterIsPerson( + personId: string, + permissions: PersonPermissions, + ): Promise>> { + if (personId == permissions.personFields.id) { + let person: Option> = await this.findById(personId); + if (!person) return { ok: false, error: new EntityNotFoundError('Person') }; + person = await this.extendPersonWithKeycloakData(person); + return { ok: true, value: person }; + } + + return this.getPersonIfAllowed(personId, permissions); + } + public async extendPersonWithKeycloakData(person: Person): Promise> { if (!person.keycloakUserId) { return person; diff --git a/src/modules/person/person-api.module.ts b/src/modules/person/person-api.module.ts index fd95185d6..0b30cfa18 100644 --- a/src/modules/person/person-api.module.ts +++ b/src/modules/person/person-api.module.ts @@ -13,10 +13,12 @@ import { PersonInfoController } from './api/person-info.controller.js'; import { PersonApiMapper } from './mapper/person-api.mapper.js'; import { PersonDeleteModule } from './person-deletion/person-delete.module.js'; import { EmailModule } from '../email/email.module.js'; +import { LdapModule } from '../../core/ldap/ldap.module.js'; @Module({ imports: [ PersonModule, + LdapModule, EmailModule, RolleModule, OrganisationModule, diff --git a/src/shared/events/email-address-changed.event.ts b/src/shared/events/email-address-changed.event.ts index 239cb009d..0d1ed6420 100644 --- a/src/shared/events/email-address-changed.event.ts +++ b/src/shared/events/email-address-changed.event.ts @@ -1,5 +1,5 @@ import { BaseEvent } from './base-event.js'; -import { EmailAddressID, PersonID } from '../types/index.js'; +import { EmailAddressID, PersonID, PersonReferrer } from '../types/index.js'; /** * This event should be triggered when an existing email-address is deactivated for a user and persisted successfully in the database and @@ -10,6 +10,7 @@ import { EmailAddressID, PersonID } from '../types/index.js'; export class EmailAddressChangedEvent extends BaseEvent { public constructor( public readonly personId: PersonID, + public readonly referrer: PersonReferrer, public readonly oldEmailAddressId: EmailAddressID, public readonly oldAddress: string, public readonly newEmailAddressId: EmailAddressID, diff --git a/src/shared/events/email-address-generated.event.ts b/src/shared/events/email-address-generated.event.ts index a602fc47a..cc8b1d45a 100644 --- a/src/shared/events/email-address-generated.event.ts +++ b/src/shared/events/email-address-generated.event.ts @@ -1,5 +1,5 @@ import { BaseEvent } from './base-event.js'; -import { EmailAddressID, PersonID } from '../types/index.js'; +import { EmailAddressID, PersonID, PersonReferrer } from '../types/index.js'; /** * This event should be triggered when a new email-address is generated for a user and persisted successfully in the database. @@ -9,6 +9,7 @@ import { EmailAddressID, PersonID } from '../types/index.js'; export class EmailAddressGeneratedEvent extends BaseEvent { public constructor( public readonly personId: PersonID, + public readonly referrer: PersonReferrer, public readonly emailAddressId: EmailAddressID, public readonly address: string, public readonly enabled: boolean, diff --git a/src/shared/events/ldap-person-entry-changed.event.ts b/src/shared/events/ldap-person-entry-changed.event.ts index c6eab7577..c76885658 100644 --- a/src/shared/events/ldap-person-entry-changed.event.ts +++ b/src/shared/events/ldap-person-entry-changed.event.ts @@ -6,6 +6,7 @@ export class LdapPersonEntryChangedEvent extends BaseEvent { public readonly personId: PersonID, public readonly mailPrimaryAddress?: string, public readonly mailAlternativeAddress?: string, + public readonly userPasswordChanged?: boolean, ) { super(); } diff --git a/src/shared/types/aggregate-ids.types.ts b/src/shared/types/aggregate-ids.types.ts index 3941b39b8..b5c4def13 100644 --- a/src/shared/types/aggregate-ids.types.ts +++ b/src/shared/types/aggregate-ids.types.ts @@ -3,6 +3,9 @@ import { Flavor } from './flavor.types.js'; declare const personSymbol: unique symbol; export type PersonID = Flavor; +declare const personReferrerSymbol: unique symbol; +export type PersonReferrer = Flavor; + declare const organisationSymbol: unique symbol; export type OrganisationID = Flavor; From ae19e32af9394a69903daacdf921cfd594e5bbdd Mon Sep 17 00:00:00 2001 From: "Marvin Rode (Cap)" <127723478+marode-cap@users.noreply.github.com> Date: Thu, 12 Dec 2024 12:10:42 +0100 Subject: [PATCH 6/8] SPSH-1626 itslearning correlation-IDs (#839) * Add syncID to itslearning requests and logs * Fix linter warnings * Fix coverage --- ...arning-organisations.event-handler.spec.ts | 155 +++++++++++------- ...itslearning-organisations.event-handler.ts | 82 +++++---- .../itslearning-persons.event-handler.spec.ts | 128 ++++++++++----- .../itslearning-persons.event-handler.ts | 100 ++++++----- .../itslearning-sync.event-handler.spec.ts | 76 ++++++--- .../itslearning-sync.event-handler.ts | 58 ++++--- .../itslearning/itslearning.service.spec.ts | 20 +++ .../itslearning/itslearning.service.ts | 14 +- .../repo/itslearning-group.repo.spec.ts | 40 +++-- .../repo/itslearning-group.repo.ts | 18 +- .../repo/itslearning-membership.repo.spec.ts | 40 +++-- .../repo/itslearning-membership.repo.ts | 30 +++- .../repo/itslearning-person.repo.spec.ts | 38 +++-- .../repo/itslearning-person.repo.ts | 37 +++-- src/shared/events/base-event.ts | 5 + 15 files changed, 551 insertions(+), 290 deletions(-) diff --git a/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.spec.ts b/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.spec.ts index 5a4f0d2ab..a6a0b45f6 100644 --- a/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.spec.ts +++ b/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.spec.ts @@ -68,13 +68,18 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.createKlasseEventHandler(event); - expect(itslearningGroupRepoMock.createOrUpdateGroup).toHaveBeenLastCalledWith<[CreateGroupParams]>({ - id: event.id, - name: event.name!, - parentId: event.administriertVon!, - type: 'Unspecified', - }); - expect(loggerMock.info).toHaveBeenLastCalledWith(`Klasse with ID ${event.id} created.`); + expect(itslearningGroupRepoMock.createOrUpdateGroup).toHaveBeenLastCalledWith<[CreateGroupParams, string]>( + { + id: event.id, + name: event.name!, + parentId: event.administriertVon!, + type: 'Unspecified', + }, + `${event.eventID}-KLASSE-CREATED`, + ); + expect(loggerMock.info).toHaveBeenLastCalledWith( + `[EventID: ${event.eventID}] Klasse with ID ${event.id} created.`, + ); }); it('should skip event, if not enabled', async () => { @@ -87,7 +92,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.createKlasseEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); expect(itslearningGroupRepoMock.createOrUpdateGroup).not.toHaveBeenCalled(); }); @@ -100,7 +105,9 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.createKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith('Klasse has no parent organisation. Aborting.'); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Klasse has no parent organisation. Aborting.`, + ); }); it('should log error, if the klasse has no name', async () => { @@ -112,7 +119,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.createKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith('Klasse has no name. Aborting.'); + expect(loggerMock.error).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Klasse has no name. Aborting.`); }); it('should log info, if the parent school is not itslearning enabled', async () => { @@ -126,7 +133,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.createKlasseEventHandler(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, + `[EventID: ${event.eventID}] Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, ); }); @@ -142,7 +149,9 @@ describe('ItsLearning Organisations Event Handler', () => { ); // CreateGroupAction await sut.createKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenLastCalledWith('Could not create Klasse in itsLearning: Error'); + expect(loggerMock.error).toHaveBeenLastCalledWith( + `[EventID: ${event.eventID}] Could not create Klasse in itsLearning: Error`, + ); }); }); @@ -157,13 +166,18 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.updatedKlasseEventHandler(event); - expect(itslearningGroupRepoMock.createOrUpdateGroup).toHaveBeenLastCalledWith<[CreateGroupParams]>({ - id: event.organisationId, - name: event.name, - parentId: event.administriertVon!, - type: 'Unspecified', - }); - expect(loggerMock.info).toHaveBeenLastCalledWith(`Klasse with ID ${event.organisationId} was updated.`); + expect(itslearningGroupRepoMock.createOrUpdateGroup).toHaveBeenLastCalledWith<[CreateGroupParams, string]>( + { + id: event.organisationId, + name: event.name, + parentId: event.administriertVon!, + type: 'Unspecified', + }, + `${event.eventID}-KLASSE-UPDATED`, + ); + expect(loggerMock.info).toHaveBeenLastCalledWith( + `[EventID: ${event.eventID}] Klasse with ID ${event.organisationId} was updated.`, + ); }); it('should skip event, if not enabled', async () => { @@ -176,7 +190,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.updatedKlasseEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); expect(itslearningGroupRepoMock.createOrUpdateGroup).not.toHaveBeenCalled(); }); @@ -189,7 +203,9 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.updatedKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith('Klasse has no parent organisation. Aborting.'); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Klasse has no parent organisation. Aborting.`, + ); expect(itslearningGroupRepoMock.createOrUpdateGroup).not.toHaveBeenCalled(); }); @@ -198,7 +214,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.updatedKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith('Klasse has no name. Aborting.'); + expect(loggerMock.error).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Klasse has no name. Aborting.`); expect(itslearningGroupRepoMock.createOrUpdateGroup).not.toHaveBeenCalled(); }); @@ -213,7 +229,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.updatedKlasseEventHandler(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, + `[EventID: ${event.eventID}] Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, ); }); @@ -230,7 +246,9 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.updatedKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenLastCalledWith('Could not update Klasse in itsLearning: Error'); + expect(loggerMock.error).toHaveBeenLastCalledWith( + `[EventID: ${event.eventID}] Could not update Klasse in itsLearning: Error`, + ); }); }); @@ -241,8 +259,13 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.deletedKlasseEventHandler(event); - expect(itslearningGroupRepoMock.deleteGroup).toHaveBeenLastCalledWith(event.organisationId); - expect(loggerMock.info).toHaveBeenLastCalledWith(`Klasse with ID ${event.organisationId} was deleted.`); + expect(itslearningGroupRepoMock.deleteGroup).toHaveBeenLastCalledWith( + event.organisationId, + `${event.eventID}-KLASSE-DELETED`, + ); + expect(loggerMock.info).toHaveBeenLastCalledWith( + `[EventID: ${event.eventID}] Klasse with ID ${event.organisationId} was deleted.`, + ); }); it('should skip event, if not enabled', async () => { @@ -251,7 +274,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.deletedKlasseEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); expect(itslearningGroupRepoMock.deleteGroup).not.toHaveBeenCalled(); }); @@ -261,7 +284,9 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.deletedKlasseEventHandler(event); - expect(loggerMock.error).toHaveBeenLastCalledWith('Could not delete Klasse in itsLearning: Error'); + expect(loggerMock.error).toHaveBeenLastCalledWith( + `[EventID: ${event.eventID}] Could not delete Klasse in itsLearning: Error`, + ); }); }); @@ -277,7 +302,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.schuleItslearningEnabledEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); expect(itslearningGroupRepoMock.createOrUpdateGroups).not.toHaveBeenCalled(); }); @@ -292,7 +317,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.schuleItslearningEnabledEventHandler(event); expect(loggerMock.error).toHaveBeenCalledWith( - `The organisation with ID ${event.organisationId} is not of type "SCHULE"!`, + `[EventID: ${event.eventID}] The organisation with ID ${event.organisationId} is not of type "SCHULE"!`, ); expect(itslearningGroupRepoMock.createOrUpdateGroups).not.toHaveBeenCalled(); }); @@ -310,7 +335,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.schuleItslearningEnabledEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith('Ersatzschule, ignoring.'); + expect(loggerMock.error).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Ersatzschule, ignoring.`); expect(itslearningGroupRepoMock.createOrUpdateGroups).not.toHaveBeenCalled(); }); @@ -332,7 +357,7 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.schuleItslearningEnabledEventHandler(event); expect(loggerMock.error).toHaveBeenLastCalledWith( - `Could not create Schule (ID ${event.organisationId}) and its Klassen in itsLearning: Error`, + `[EventID: ${event.eventID}] Could not create Schule (ID ${event.organisationId}) and its Klassen in itsLearning: Error`, ); }); @@ -360,28 +385,31 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.schuleItslearningEnabledEventHandler(event); expect(loggerMock.info).toHaveBeenLastCalledWith( - `Schule with ID ${event.organisationId} and its 2 Klassen were created.`, + `[EventID: ${event.eventID}] Schule with ID ${event.organisationId} and its 2 Klassen were created.`, + ); + expect(itslearningGroupRepoMock.createOrUpdateGroups).toHaveBeenCalledWith<[CreateGroupParams[], string]>( + [ + { + id: event.organisationId, + name: `${event.kennung} (${event.name})`, + type: 'School', + parentId: sut.ROOT_OEFFENTLICH, + }, + { + id: klasse1.id, + name: `${klasse1.name}`, + type: 'Unspecified', + parentId: event.organisationId, + }, + { + id: klasse2.id, + name: `Unbenannte Klasse`, + type: 'Unspecified', + parentId: event.organisationId, + }, + ], + `${event.eventID}-SCHULE-SYNC`, ); - expect(itslearningGroupRepoMock.createOrUpdateGroups).toHaveBeenCalledWith<[CreateGroupParams[]]>([ - { - id: event.organisationId, - name: `${event.kennung} (${event.name})`, - type: 'School', - parentId: sut.ROOT_OEFFENTLICH, - }, - { - id: klasse1.id, - name: `${klasse1.name}`, - type: 'Unspecified', - parentId: event.organisationId, - }, - { - id: klasse2.id, - name: `Unbenannte Klasse`, - type: 'Unspecified', - parentId: event.organisationId, - }, - ]); }); it('should set default name for schule', async () => { @@ -400,16 +428,19 @@ describe('ItsLearning Organisations Event Handler', () => { await sut.schuleItslearningEnabledEventHandler(event); expect(loggerMock.info).toHaveBeenLastCalledWith( - `Schule with ID ${event.organisationId} and its 0 Klassen were created.`, + `[EventID: ${event.eventID}] Schule with ID ${event.organisationId} and its 0 Klassen were created.`, + ); + expect(itslearningGroupRepoMock.createOrUpdateGroups).toHaveBeenCalledWith<[CreateGroupParams[], string]>( + [ + { + id: event.organisationId, + name: `${event.kennung} (Unbenannte Schule)`, + type: 'School', + parentId: sut.ROOT_OEFFENTLICH, + }, + ], + `${event.eventID}-SCHULE-SYNC`, ); - expect(itslearningGroupRepoMock.createOrUpdateGroups).toHaveBeenCalledWith<[CreateGroupParams[]]>([ - { - id: event.organisationId, - name: `${event.kennung} (Unbenannte Schule)`, - type: 'School', - parentId: sut.ROOT_OEFFENTLICH, - }, - ]); }); }); }); diff --git a/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.ts b/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.ts index 60734b4c5..e0c792fe9 100644 --- a/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.ts +++ b/src/modules/itslearning/event-handlers/itslearning-organisations.event-handler.ts @@ -38,19 +38,19 @@ export class ItsLearningOrganisationsEventHandler { @EventHandler(KlasseCreatedEvent) public async createKlasseEventHandler(event: KlasseCreatedEvent): Promise { - this.logger.info(`Received KlasseCreatedEvent, ID: ${event.id}`); + this.logger.info(`[EventID: ${event.eventID}] Received KlasseCreatedEvent, ID: ${event.id}`); if (!this.ENABLED) { - this.logger.info('Not enabled, ignoring event.'); + this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); return; } if (!event.administriertVon) { - return this.logger.error('Klasse has no parent organisation. Aborting.'); + return this.logger.error(`[EventID: ${event.eventID}] Klasse has no parent organisation. Aborting.`); } if (!event.name) { - return this.logger.error('Klasse has no name. Aborting.'); + return this.logger.error(`[EventID: ${event.eventID}] Klasse has no name. Aborting.`); } { @@ -58,7 +58,7 @@ export class ItsLearningOrganisationsEventHandler { const parent: Option> = await this.organisationRepo.findById(event.administriertVon); if (!parent?.itslearningEnabled) { return this.logger.info( - `Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, + `[EventID: ${event.eventID}] Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, ); } } @@ -70,30 +70,37 @@ export class ItsLearningOrganisationsEventHandler { parentId: event.administriertVon, }; - const createError: Option = await this.itslearningGroupRepo.createOrUpdateGroup(params); + const createError: Option = await this.itslearningGroupRepo.createOrUpdateGroup( + params, + `${event.eventID}-KLASSE-CREATED`, + ); if (createError) { - return this.logger.error(`Could not create Klasse in itsLearning: ${createError.message}`); + return this.logger.error( + `[EventID: ${event.eventID}] Could not create Klasse in itsLearning: ${createError.message}`, + ); } - this.logger.info(`Klasse with ID ${event.id} created.`); + this.logger.info(`[EventID: ${event.eventID}] Klasse with ID ${event.id} created.`); } @EventHandler(KlasseUpdatedEvent) public async updatedKlasseEventHandler(event: KlasseUpdatedEvent): Promise { - this.logger.info(`Received KlasseUpdatedEvent, ID: ${event.organisationId}, new name: ${event.name}`); + this.logger.info( + `[EventID: ${event.eventID}] Received KlasseUpdatedEvent, ID: ${event.organisationId}, new name: ${event.name}`, + ); if (!this.ENABLED) { - this.logger.info('Not enabled, ignoring event.'); + this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); return; } if (!event.administriertVon) { - return this.logger.error('Klasse has no parent organisation. Aborting.'); + return this.logger.error(`[EventID: ${event.eventID}] Klasse has no parent organisation. Aborting.`); } if (!event.name) { - return this.logger.error('Klasse has no name. Aborting.'); + return this.logger.error(`[EventID: ${event.eventID}] Klasse has no name. Aborting.`); } { @@ -101,7 +108,7 @@ export class ItsLearningOrganisationsEventHandler { const parent: Option> = await this.organisationRepo.findById(event.administriertVon); if (!parent?.itslearningEnabled) { return this.logger.info( - `Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, + `[EventID: ${event.eventID}] Parent Organisation (${event.administriertVon}) is not an itslearning schule.`, ); } } @@ -113,44 +120,58 @@ export class ItsLearningOrganisationsEventHandler { parentId: event.administriertVon, }; - const createError: Option = await this.itslearningGroupRepo.createOrUpdateGroup(params); + const createError: Option = await this.itslearningGroupRepo.createOrUpdateGroup( + params, + `${event.eventID}-KLASSE-UPDATED`, + ); if (createError) { - return this.logger.error(`Could not update Klasse in itsLearning: ${createError.message}`); + return this.logger.error( + `[EventID: ${event.eventID}] Could not update Klasse in itsLearning: ${createError.message}`, + ); } - this.logger.info(`Klasse with ID ${event.organisationId} was updated.`); + this.logger.info(`[EventID: ${event.eventID}] Klasse with ID ${event.organisationId} was updated.`); } @EventHandler(KlasseDeletedEvent) public async deletedKlasseEventHandler(event: KlasseDeletedEvent): Promise { - this.logger.info(`Received KlasseUpdatedEvent, ID: ${event.organisationId}`); + this.logger.info(`[EventID: ${event.eventID}] Received KlasseUpdatedEvent, ID: ${event.organisationId}`); if (!this.ENABLED) { - this.logger.info('Not enabled, ignoring event.'); + this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); return; } - const deleteError: Option = await this.itslearningGroupRepo.deleteGroup(event.organisationId); + const deleteError: Option = await this.itslearningGroupRepo.deleteGroup( + event.organisationId, + `${event.eventID}-KLASSE-DELETED`, + ); if (deleteError) { - return this.logger.error(`Could not delete Klasse in itsLearning: ${deleteError.message}`); + return this.logger.error( + `[EventID: ${event.eventID}] Could not delete Klasse in itsLearning: ${deleteError.message}`, + ); } - this.logger.info(`Klasse with ID ${event.organisationId} was deleted.`); + this.logger.info(`[EventID: ${event.eventID}] Klasse with ID ${event.organisationId} was deleted.`); } @EventHandler(SchuleItslearningEnabledEvent) public async schuleItslearningEnabledEventHandler(event: SchuleItslearningEnabledEvent): Promise { - this.logger.info(`Received EnableSchuleItslearningEvent, ID: ${event.organisationId}`); + this.logger.info( + `[EventID: ${event.eventID}] Received EnableSchuleItslearningEvent, ID: ${event.organisationId}`, + ); if (!this.ENABLED) { - this.logger.info('Not enabled, ignoring event.'); + this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); return; } if (event.typ !== OrganisationsTyp.SCHULE) { - this.logger.error(`The organisation with ID ${event.organisationId} is not of type "SCHULE"!`); + this.logger.error( + `[EventID: ${event.eventID}] The organisation with ID ${event.organisationId} is not of type "SCHULE"!`, + ); return; } @@ -160,7 +181,7 @@ export class ItsLearningOrganisationsEventHandler { ]); if (rootType === RootDirectChildrenType.ERSATZ) { - this.logger.error('Ersatzschule, ignoring.'); + this.logger.error(`[EventID: ${event.eventID}] Ersatzschule, ignoring.`); return; } @@ -182,14 +203,19 @@ export class ItsLearningOrganisationsEventHandler { parentId: this.ROOT_OEFFENTLICH, }); - const createError: Option = await this.itslearningGroupRepo.createOrUpdateGroups(createParams); + const createError: Option = await this.itslearningGroupRepo.createOrUpdateGroups( + createParams, + `${event.eventID}-SCHULE-SYNC`, + ); if (createError) { return this.logger.error( - `Could not create Schule (ID ${event.organisationId}) and its Klassen in itsLearning: ${createError.message}`, + `[EventID: ${event.eventID}] Could not create Schule (ID ${event.organisationId}) and its Klassen in itsLearning: ${createError.message}`, ); } - this.logger.info(`Schule with ID ${event.organisationId} and its ${klassen.length} Klassen were created.`); + this.logger.info( + `[EventID: ${event.eventID}] Schule with ID ${event.organisationId} and its ${klassen.length} Klassen were created.`, + ); } } diff --git a/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.spec.ts b/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.spec.ts index 9782d6ae6..a6f3a7a8a 100644 --- a/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.spec.ts +++ b/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.spec.ts @@ -86,12 +86,13 @@ describe('ItsLearning Persons Event Handler', () => { ok: true, value: { deleted: 0, updated: 1 }, } satisfies Result); + const eventID: string = faker.string.uuid(); - await sut.updateMemberships(personId, currentKontexte); + await sut.updateMemberships(personId, currentKontexte, eventID); expect(itslearningMembershipRepoMock.setMemberships).toHaveBeenCalledTimes(1); expect(loggerMock.info).toHaveBeenCalledWith( - `Set ${currentKontexte.length} memberships for person ${personId}`, + `[EventID: ${eventID}] Set ${currentKontexte.length} memberships for person ${personId}`, ); }); @@ -103,12 +104,13 @@ describe('ItsLearning Persons Event Handler', () => { SetMembershipsResult, DomainError >); + const eventID: string = faker.string.uuid(); - await sut.updateMemberships(personId, currentKontexte); + await sut.updateMemberships(personId, currentKontexte, eventID); expect(itslearningMembershipRepoMock.setMemberships).toHaveBeenCalledTimes(1); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not set ${currentKontexte.length} memberships for person ${personId}`, + `[EventID: ${eventID}] Could not set ${currentKontexte.length} memberships for person ${personId}`, error, ); }); @@ -118,22 +120,24 @@ describe('ItsLearning Persons Event Handler', () => { it('should delete person in itsLearning', async () => { const personID: string = faker.string.uuid(); itslearningPersonRepoMock.deletePerson.mockResolvedValueOnce(undefined); + const eventID: string = faker.string.uuid(); - await sut.deletePerson(personID); + await sut.deletePerson(personID, eventID); expect(itslearningPersonRepoMock.deletePerson).toHaveBeenCalledWith(personID); - expect(loggerMock.info).toHaveBeenCalledWith(`Person with ID ${personID} deleted.`); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${eventID}] Person with ID ${personID} deleted.`); }); it('should log error if person could not be deleted', async () => { const personID: string = faker.string.uuid(); itslearningPersonRepoMock.deletePerson.mockResolvedValueOnce(new ItsLearningError('Test Error')); + const eventID: string = faker.string.uuid(); - await sut.deletePerson(personID); + await sut.deletePerson(personID, eventID); expect(itslearningPersonRepoMock.deletePerson).toHaveBeenCalledWith(personID); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not delete person with ID ${personID} from itsLearning.`, + `[EventID: ${eventID}] Could not delete person with ID ${personID} from itsLearning.`, ); }); }); @@ -161,38 +165,48 @@ describe('ItsLearning Persons Event Handler', () => { const [person, personResponse]: [Person, PersonResponse] = createPersonAndResponse(); itslearningPersonRepoMock.readPerson.mockResolvedValueOnce(personResponse); // Read person itslearningPersonRepoMock.createOrUpdatePerson.mockResolvedValueOnce(undefined); // Create person + const event: PersonRenamedEvent = PersonRenamedEvent.fromPerson(person, faker.internet.userName()); - await sut.personRenamedEventHandler(PersonRenamedEvent.fromPerson(person, faker.internet.userName())); + await sut.personRenamedEventHandler(event); - expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledWith({ - id: person.id, - firstName: person.vorname, - lastName: person.familienname, - username: person.referrer, - institutionRoleType: personResponse.institutionRole, - }); - expect(loggerMock.info).toHaveBeenCalledWith(`Person with ID ${person.id} updated in itsLearning!`); + expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledWith( + { + id: person.id, + firstName: person.vorname, + lastName: person.familienname, + username: person.referrer, + institutionRoleType: personResponse.institutionRole, + }, + `${event.eventID}-PERSON-RENAMED-UPDATE`, + ); + expect(loggerMock.info).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Person with ID ${person.id} updated in itsLearning!`, + ); }); it('should log error if person could not be updated', async () => { const [person, personResponse]: [Person, PersonResponse] = createPersonAndResponse(); itslearningPersonRepoMock.readPerson.mockResolvedValueOnce(personResponse); // Read person itslearningPersonRepoMock.createOrUpdatePerson.mockResolvedValueOnce(new ItsLearningError('Test Error')); // Create person + const event: PersonRenamedEvent = PersonRenamedEvent.fromPerson(person, faker.internet.userName()); - await sut.personRenamedEventHandler(PersonRenamedEvent.fromPerson(person, faker.internet.userName())); + await sut.personRenamedEventHandler(event); expect(loggerMock.error).toHaveBeenCalledWith( - `Person with ID ${person.id} could not be updated in itsLearning!`, + `[EventID: ${event.eventID}] Person with ID ${person.id} could not be updated in itsLearning!`, ); }); describe('when person is invalid', () => { it('should log error, if person has no referrer', async () => { const [person]: [Person, PersonResponse] = createPersonAndResponse({ referrer: undefined }); + const event: PersonRenamedEvent = PersonRenamedEvent.fromPerson(person, faker.internet.userName()); - await sut.personRenamedEventHandler(PersonRenamedEvent.fromPerson(person, faker.internet.userName())); + await sut.personRenamedEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith(`Person with ID ${person.id} has no username!`); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Person with ID ${person.id} has no username!`, + ); }); }); @@ -200,11 +214,12 @@ describe('ItsLearning Persons Event Handler', () => { it('should log info', async () => { const [person]: [Person, PersonResponse] = createPersonAndResponse(); itslearningPersonRepoMock.readPerson.mockResolvedValueOnce(undefined); // Read person + const event: PersonRenamedEvent = PersonRenamedEvent.fromPerson(person, faker.internet.userName()); - await sut.personRenamedEventHandler(PersonRenamedEvent.fromPerson(person, faker.internet.userName())); + await sut.personRenamedEventHandler(event); expect(loggerMock.info).toHaveBeenCalledWith( - `Person with ID ${person.id} is not in itslearning, ignoring.`, + `[EventID: ${event.eventID}] Person with ID ${person.id} is not in itslearning, ignoring.`, ); }); }); @@ -212,10 +227,11 @@ describe('ItsLearning Persons Event Handler', () => { it('should skip event, if not enabled', async () => { sut.ENABLED = false; const [person]: [Person, PersonResponse] = createPersonAndResponse(); + const event: PersonRenamedEvent = PersonRenamedEvent.fromPerson(person, faker.internet.userName()); - await sut.personRenamedEventHandler(PersonRenamedEvent.fromPerson(person, faker.internet.userName())); + await sut.personRenamedEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); }); }); @@ -230,27 +246,34 @@ describe('ItsLearning Persons Event Handler', () => { it('should send person to itsLearning', async () => { const kontextData: PersonenkontextUpdatedData = makeKontextEventData({ rolle: RollenArt.LERN }); itslearningPersonRepoMock.createOrUpdatePerson.mockResolvedValueOnce(undefined); + const eventID: string = faker.string.uuid(); - await sut.updatePerson(person, [kontextData]); + await sut.updatePerson(person, [kontextData], eventID); - expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledWith({ - id: person.id, - firstName: person.vorname, - lastName: person.familienname, - username: person.referrer, - institutionRoleType: IMSESInstitutionRoleType.STUDENT, - }); - expect(loggerMock.info).toHaveBeenCalledWith(`Person with ID ${person.id} created in itsLearning!`); + expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledWith( + { + id: person.id, + firstName: person.vorname, + lastName: person.familienname, + username: person.referrer, + institutionRoleType: IMSESInstitutionRoleType.STUDENT, + }, + eventID, + ); + expect(loggerMock.info).toHaveBeenCalledWith( + `[EventID: ${eventID}] Person with ID ${person.id} created in itsLearning!`, + ); }); it('should log error if person could not be created', async () => { const kontextData: PersonenkontextUpdatedData = makeKontextEventData({ rolle: RollenArt.LERN }); itslearningPersonRepoMock.createOrUpdatePerson.mockResolvedValueOnce(new ItsLearningError('Test Error')); + const eventID: string = faker.string.uuid(); - await sut.updatePerson(person, [kontextData]); + await sut.updatePerson(person, [kontextData], eventID); expect(loggerMock.error).toHaveBeenCalledWith( - `Person with ID ${person.id} could not be sent to itsLearning! Error: Test Error`, + `[EventID: ${eventID}] Person with ID ${person.id} could not be sent to itsLearning! Error: Test Error`, ); }); @@ -258,10 +281,13 @@ describe('ItsLearning Persons Event Handler', () => { it('should log error, if person has no referrer', async () => { // eslint-disable-next-line @typescript-eslint/no-unused-vars const { referrer, ...personWithoutReferrer }: PersonenkontextUpdatedPersonData = person; + const eventID: string = faker.string.uuid(); - await sut.updatePerson(personWithoutReferrer, [createMock()]); + await sut.updatePerson(personWithoutReferrer, [createMock()], eventID); - expect(loggerMock.error).toHaveBeenCalledWith(`Person with ID ${person.id} has no username!`); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${eventID}] Person with ID ${person.id} has no username!`, + ); }); }); }); @@ -284,8 +310,14 @@ describe('ItsLearning Persons Event Handler', () => { await sut.oxUserChangedEventHandler(generatedEvent); - expect(itslearningPersonRepoMock.updateEmail).toHaveBeenCalledWith(personId, email); - expect(loggerMock.info).toHaveBeenCalledWith(`Updated E-Mail for person with ID ${personId}!`); + expect(itslearningPersonRepoMock.updateEmail).toHaveBeenCalledWith( + personId, + email, + `${generatedEvent.eventID}-EMAIL-UPDATE`, + ); + expect(loggerMock.info).toHaveBeenCalledWith( + `[EventID: ${generatedEvent.eventID}] Updated E-Mail for person with ID ${personId}!`, + ); }); it('should log error, if email could not be updated', async () => { @@ -293,7 +325,9 @@ describe('ItsLearning Persons Event Handler', () => { await sut.oxUserChangedEventHandler(generatedEvent); - expect(loggerMock.error).toHaveBeenCalledWith(`Could not update E-Mail for person with ID ${personId}!`); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${generatedEvent.eventID}] Could not update E-Mail for person with ID ${personId}!`, + ); }); it('should skip event, if not enabled', async () => { @@ -302,7 +336,9 @@ describe('ItsLearning Persons Event Handler', () => { await sut.oxUserChangedEventHandler(generatedEvent); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring email update.'); + expect(loggerMock.info).toHaveBeenCalledWith( + `[EventID: ${generatedEvent.eventID}] Not enabled, ignoring email update.`, + ); }); }); @@ -366,7 +402,11 @@ describe('ItsLearning Persons Event Handler', () => { await sut.updatePersonenkontexteEventHandler(event); expect(updateMembershipsSpy).toHaveBeenCalledTimes(1); - expect(updateMembershipsSpy).toHaveBeenCalledWith(event.person.id, expect.objectContaining({ length: 2 })); + expect(updateMembershipsSpy).toHaveBeenCalledWith( + event.person.id, + expect.objectContaining({ length: 2 }), + `${event.eventID}-UPDATE-MEMBERSHIPS`, + ); }); it('should not call updatePerson, if no relevant kontext exists', async () => { @@ -412,7 +452,7 @@ describe('ItsLearning Persons Event Handler', () => { await sut.updatePersonenkontexteEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); }); }); }); diff --git a/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.ts b/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.ts index 796ee8e44..19cf2f161 100644 --- a/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.ts +++ b/src/modules/itslearning/event-handlers/itslearning-persons.event-handler.ts @@ -43,58 +43,71 @@ export class ItsLearningPersonsEventHandler { @EventHandler(PersonRenamedEvent) public async personRenamedEventHandler(event: PersonRenamedEvent): Promise { await this.personUpdateMutex.runExclusive(async () => { - this.logger.info(`Received PersonRenamedEvent, ${event.personId}`); + this.logger.info(`[EventID: ${event.eventID}] Received PersonRenamedEvent, ${event.personId}`); if (!this.ENABLED) { - return this.logger.info('Not enabled, ignoring event.'); + return this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); } if (!event.referrer) { - return this.logger.error(`Person with ID ${event.personId} has no username!`); + return this.logger.error( + `[EventID: ${event.eventID}] Person with ID ${event.personId} has no username!`, + ); } const readPersonResult: Option = await this.itslearningPersonRepo.readPerson( event.personId, + `${event.eventID}-PERSON-EXISTS-CHECK`, ); if (!readPersonResult) { - return this.logger.info(`Person with ID ${event.personId} is not in itslearning, ignoring.`); + return this.logger.info( + `[EventID: ${event.eventID}] Person with ID ${event.personId} is not in itslearning, ignoring.`, + ); } - const updatePersonError: Option = await this.itslearningPersonRepo.createOrUpdatePerson({ - id: event.personId, - firstName: event.vorname, - lastName: event.familienname, - username: event.referrer, - institutionRoleType: readPersonResult.institutionRole, - }); + const updatePersonError: Option = await this.itslearningPersonRepo.createOrUpdatePerson( + { + id: event.personId, + firstName: event.vorname, + lastName: event.familienname, + username: event.referrer, + institutionRoleType: readPersonResult.institutionRole, + }, + `${event.eventID}-PERSON-RENAMED-UPDATE`, + ); if (updatePersonError) { - return this.logger.error(`Person with ID ${event.personId} could not be updated in itsLearning!`); + return this.logger.error( + `[EventID: ${event.eventID}] Person with ID ${event.personId} could not be updated in itsLearning!`, + ); } - this.logger.info(`Person with ID ${event.personId} updated in itsLearning!`); + this.logger.info(`[EventID: ${event.eventID}] Person with ID ${event.personId} updated in itsLearning!`); }); } @EventHandler(OxUserChangedEvent) public async oxUserChangedEventHandler(event: OxUserChangedEvent): Promise { if (!this.ENABLED) { - return this.logger.info('Not enabled, ignoring email update.'); + return this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring email update.`); } await this.personUpdateMutex.runExclusive(async () => { - this.logger.info(`Received OxUserChangedEvent, ${event.personId}`); + this.logger.info(`[EventID: ${event.eventID}] Received OxUserChangedEvent, ${event.personId}`); const updateError: Option = await this.itslearningPersonRepo.updateEmail( event.personId, event.primaryEmail, + `${event.eventID}-EMAIL-UPDATE`, ); if (updateError) { - this.logger.error(`Could not update E-Mail for person with ID ${event.personId}!`); + this.logger.error( + `[EventID: ${event.eventID}] Could not update E-Mail for person with ID ${event.personId}!`, + ); } else { - this.logger.info(`Updated E-Mail for person with ID ${event.personId}!`); + this.logger.info(`[EventID: ${event.eventID}] Updated E-Mail for person with ID ${event.personId}!`); } }); } @@ -102,10 +115,10 @@ export class ItsLearningPersonsEventHandler { @EventHandler(PersonenkontextUpdatedEvent) public async updatePersonenkontexteEventHandler(event: PersonenkontextUpdatedEvent): Promise { await this.personUpdateMutex.runExclusive(async () => { - this.logger.info(`Received PersonenkontextUpdatedEvent, ${event.person.id}`); + this.logger.info(`[EventID: ${event.eventID}] Received PersonenkontextUpdatedEvent, ${event.person.id}`); if (!this.ENABLED) { - return this.logger.info('Not enabled, ignoring event.'); + return this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); } // Collect all itslearning-orgas @@ -123,30 +136,35 @@ export class ItsLearningPersonsEventHandler { if (currentKontexte.length > 0) { // Person should have itslearning, create/update them as necessary - await this.updatePerson(event.person, currentKontexte); + await this.updatePerson(event.person, currentKontexte, `${event.eventID}-UPDATE-PERSON`); // Synchronize memberships - await this.updateMemberships(event.person.id, currentKontexte); + await this.updateMemberships(event.person.id, currentKontexte, `${event.eventID}-UPDATE-MEMBERSHIPS`); } else { // Delete person - await this.deletePerson(event.person.id); + await this.deletePerson(event.person.id, `${event.eventID}-DELETE-PERSON`); } }); } - public async updateMemberships(personId: PersonID, currentKontexte: PersonenkontextUpdatedData[]): Promise { + public async updateMemberships( + personId: PersonID, + currentKontexte: PersonenkontextUpdatedData[], + eventID: string, + ): Promise { const setMembershipsResult: Result = await this.itslearningMembershipRepo.setMemberships( personId, currentKontexte.map((pk: PersonenkontextUpdatedData) => ({ organisationId: pk.orgaId, role: pk.rolle })), + eventID, ); if (!setMembershipsResult.ok) { this.logger.error( - `Could not set ${currentKontexte.length} memberships for person ${personId}`, + `[EventID: ${eventID}] Could not set ${currentKontexte.length} memberships for person ${personId}`, setMembershipsResult.error, ); } else { - this.logger.info(`Set ${currentKontexte.length} memberships for person ${personId}`); + this.logger.info(`[EventID: ${eventID}] Set ${currentKontexte.length} memberships for person ${personId}`); } } @@ -156,43 +174,47 @@ export class ItsLearningPersonsEventHandler { public async updatePerson( person: PersonenkontextUpdatedPersonData, currentPersonenkontexte: PersonenkontextUpdatedData[], + eventID: string, ): Promise { if (!person.referrer) { - return this.logger.error(`Person with ID ${person.id} has no username!`); + return this.logger.error(`[EventID: ${eventID}] Person with ID ${person.id} has no username!`); } const targetRole: IMSESInstitutionRoleType = rollenartToIMSESInstitutionRole( determineHighestRollenart(currentPersonenkontexte.map((pk: PersonenkontextUpdatedData) => pk.rolle)), ); - const createError: Option = await this.itslearningPersonRepo.createOrUpdatePerson({ - id: person.id, - firstName: person.vorname, - lastName: person.familienname, - username: person.referrer, - institutionRoleType: targetRole, - email: person.email, - }); + const createError: Option = await this.itslearningPersonRepo.createOrUpdatePerson( + { + id: person.id, + firstName: person.vorname, + lastName: person.familienname, + username: person.referrer, + institutionRoleType: targetRole, + email: person.email, + }, + eventID, + ); if (createError) { return this.logger.error( - `Person with ID ${person.id} could not be sent to itsLearning! Error: ${createError.message}`, + `[EventID: ${eventID}] Person with ID ${person.id} could not be sent to itsLearning! Error: ${createError.message}`, ); } - return this.logger.info(`Person with ID ${person.id} created in itsLearning!`); + return this.logger.info(`[EventID: ${eventID}] Person with ID ${person.id} created in itsLearning!`); } /** * Delete this person in itslearning */ - public async deletePerson(personID: PersonID): Promise { + public async deletePerson(personID: PersonID, eventID: string): Promise { const deleteError: Option = await this.itslearningPersonRepo.deletePerson(personID); if (!deleteError) { - this.logger.info(`Person with ID ${personID} deleted.`); + this.logger.info(`[EventID: ${eventID}] Person with ID ${personID} deleted.`); } else { - this.logger.error(`Could not delete person with ID ${personID} from itsLearning.`); + this.logger.error(`[EventID: ${eventID}] Could not delete person with ID ${personID} from itsLearning.`); } } diff --git a/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.spec.ts b/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.spec.ts index 3906fb340..29b6571e0 100644 --- a/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.spec.ts +++ b/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.spec.ts @@ -167,16 +167,22 @@ describe('ItsLearning Persons Event Handler', () => { it('should create or update user', async () => { itslearningPersonRepoMock.createOrUpdatePerson.mockResolvedValueOnce(undefined); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(person.id)); - - expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledWith({ - id: person.id, - firstName: person.vorname, - lastName: person.familienname, - username: person.referrer, - institutionRoleType: rollenartToIMSESInstitutionRole(rolleWithItslearning.rollenart), - }); - expect(loggerMock.info).toHaveBeenCalledWith(`Updated person with ID ${person.id} in itslearning!`); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(person.id); + await sut.personExternalSystemSyncEventHandler(event); + + expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledWith( + { + id: person.id, + firstName: person.vorname, + lastName: person.familienname, + username: person.referrer, + institutionRoleType: rollenartToIMSESInstitutionRole(rolleWithItslearning.rollenart), + }, + `${event.eventID}-SYNC-PERSON`, + ); + expect(loggerMock.info).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Updated person with ID ${person.id} in itslearning!`, + ); }); it('should log error if creation failed', async () => { @@ -184,11 +190,12 @@ describe('ItsLearning Persons Event Handler', () => { new ItsLearningError('Error Test'), ); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(person.id)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(person.id); + await sut.personExternalSystemSyncEventHandler(event); expect(itslearningPersonRepoMock.createOrUpdatePerson).toHaveBeenCalledTimes(1); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not create/update person with ID ${person.id} in itslearning!`, + `[EventID: ${event.eventID}] Could not create/update person with ID ${person.id} in itslearning!`, ); }); @@ -199,7 +206,8 @@ describe('ItsLearning Persons Event Handler', () => { value: { deleted: 0, updated: 1 }, } satisfies Result); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(person.id)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(person.id); + await sut.personExternalSystemSyncEventHandler(event); expect(itslearningMembershipRepoMock.setMemberships).toHaveBeenCalledWith( person.id, @@ -213,9 +221,10 @@ describe('ItsLearning Persons Event Handler', () => { role: rolleWithItslearning.rollenart, }, ]), + `${event.eventID}-SYNC-PERSON-MEMBERSHIPS`, ); expect(loggerMock.info).toHaveBeenCalledWith( - `Created/Updated 1 and deleted 0 memberships for person with ID ${person.id} to itslearning!`, + `[EventID: ${event.eventID}] Created/Updated 1 and deleted 0 memberships for person with ID ${person.id} to itslearning!`, ); }); @@ -226,10 +235,11 @@ describe('ItsLearning Persons Event Handler', () => { error: new ItsLearningError('Error Test'), } satisfies Result); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(person.id)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(person.id); + await sut.personExternalSystemSyncEventHandler(event); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not delete person with ID ${person.id} from itslearning!`, + `[EventID: ${event.eventID}] Could not delete person with ID ${person.id} from itslearning!`, ); }); }); @@ -258,18 +268,23 @@ describe('ItsLearning Persons Event Handler', () => { it('should delete person', async () => { itslearningPersonRepoMock.deletePerson.mockResolvedValueOnce(undefined); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(person.id)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(person.id); + await sut.personExternalSystemSyncEventHandler(event); - expect(itslearningPersonRepoMock.deletePerson).toHaveBeenCalledWith(person.id); + expect(itslearningPersonRepoMock.deletePerson).toHaveBeenCalledWith( + person.id, + `${event.eventID}-DELETE`, + ); }); it('should log error if deletion failed', async () => { itslearningPersonRepoMock.deletePerson.mockResolvedValueOnce(new ItsLearningError('Error Test')); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(person.id)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(person.id); + await sut.personExternalSystemSyncEventHandler(event); expect(loggerMock.error).toHaveBeenCalledWith( - `Could not delete person with ID ${person.id} from itslearning!`, + `[EventID: ${event.eventID}] Could not delete person with ID ${person.id} from itslearning!`, ); }); }); @@ -279,18 +294,24 @@ describe('ItsLearning Persons Event Handler', () => { const personId: string = faker.string.uuid(); personRepoMock.findById.mockResolvedValueOnce(undefined); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(personId)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(personId); + await sut.personExternalSystemSyncEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith(`Person with ID ${personId} could not be found!`); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Person with ID ${personId} could not be found!`, + ); }); it('should log error, if person has no username', async () => { const personId: string = faker.string.uuid(); personRepoMock.findById.mockResolvedValueOnce(DoFactory.createPerson(true, { referrer: undefined })); - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(personId)); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(personId); + await sut.personExternalSystemSyncEventHandler(event); - expect(loggerMock.error).toHaveBeenCalledWith(`Person with ID ${personId} has no username!`); + expect(loggerMock.error).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Person with ID ${personId} has no username!`, + ); }); }); @@ -298,9 +319,12 @@ describe('ItsLearning Persons Event Handler', () => { it('should log info and return', async () => { sut.ENABLED = false; - await sut.personExternalSystemSyncEventHandler(new PersonExternalSystemsSyncEvent(faker.string.uuid())); + const event: PersonExternalSystemsSyncEvent = new PersonExternalSystemsSyncEvent(faker.string.uuid()); + await sut.personExternalSystemSyncEventHandler(event); - expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event.'); + expect(loggerMock.info).toHaveBeenCalledWith( + `[EventID: ${event.eventID}] Not enabled, ignoring event.`, + ); }); }); }); diff --git a/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.ts b/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.ts index af60a25dc..fa7727eb7 100644 --- a/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.ts +++ b/src/modules/itslearning/event-handlers/itslearning-sync.event-handler.ts @@ -51,21 +51,23 @@ export class ItsLearningSyncEventHandler { @EventHandler(PersonExternalSystemsSyncEvent) public async personExternalSystemSyncEventHandler(event: PersonExternalSystemsSyncEvent): Promise { - this.logger.info(`Received PersonExternalSystemsSyncEvent, ${event.personId}`); + this.logger.info(`[EventID: ${event.eventID}] Received PersonExternalSystemsSyncEvent, ${event.personId}`); if (!this.ENABLED) { - return this.logger.info('Not enabled, ignoring event.'); + return this.logger.info(`[EventID: ${event.eventID}] Not enabled, ignoring event.`); } // Retrieve the person from the DB const person: Option> = await this.personRepo.findById(event.personId); if (!person) { - return this.logger.error(`Person with ID ${event.personId} could not be found!`); + return this.logger.error( + `[EventID: ${event.eventID}] Person with ID ${event.personId} could not be found!`, + ); } // Check if person has a username if (!person.referrer) { - return this.logger.error(`Person with ID ${event.personId} has no username!`); + return this.logger.error(`[EventID: ${event.eventID}] Person with ID ${event.personId} has no username!`); } // Get all personenkontexte for this person @@ -123,20 +125,25 @@ export class ItsLearningSyncEventHandler { ); // Create or update the person in itslearning - const creationError: Option = await this.itslearningPersonRepo.createOrUpdatePerson({ - id: person.id, - firstName: person.vorname, - lastName: person.familienname, - username: person.referrer, - institutionRoleType: rollenartToIMSESInstitutionRole(targetRole), - email: person.email, - }); + const creationError: Option = await this.itslearningPersonRepo.createOrUpdatePerson( + { + id: person.id, + firstName: person.vorname, + lastName: person.familienname, + username: person.referrer, + institutionRoleType: rollenartToIMSESInstitutionRole(targetRole), + email: person.email, + }, + `${event.eventID}-SYNC-PERSON`, + ); if (creationError) { - return this.logger.error(`Could not create/update person with ID ${person.id} in itslearning!`); + return this.logger.error( + `[EventID: ${event.eventID}] Could not create/update person with ID ${person.id} in itslearning!`, + ); } - this.logger.info(`Updated person with ID ${person.id} in itslearning!`); + this.logger.info(`[EventID: ${event.eventID}] Updated person with ID ${person.id} in itslearning!`); // Set the memberships const memberships: SetMembershipParams[] = relevantKontexte.map((pk: Personenkontext) => ({ @@ -147,25 +154,36 @@ export class ItsLearningSyncEventHandler { })); const setMembershipsResult: Result = - await this.itslearningMembershipRepo.setMemberships(person.id, memberships); + await this.itslearningMembershipRepo.setMemberships( + person.id, + memberships, + `${event.eventID}-SYNC-PERSON-MEMBERSHIPS`, + ); if (!setMembershipsResult.ok) { - return this.logger.error(`Could not delete person with ID ${person.id} from itslearning!`); + return this.logger.error( + `[EventID: ${event.eventID}] Could not delete person with ID ${person.id} from itslearning!`, + ); } this.logger.info( - `Created/Updated ${setMembershipsResult.value.updated} and deleted ${setMembershipsResult.value.deleted} memberships for person with ID ${person.id} to itslearning!`, + `[EventID: ${event.eventID}] Created/Updated ${setMembershipsResult.value.updated} and deleted ${setMembershipsResult.value.deleted} memberships for person with ID ${person.id} to itslearning!`, ); } else { this.logger.info( - `Deleting person with ID ${person.id} from itslearning (if they exist), because they have no relevant personenkontexte!`, + `[EventID: ${event.eventID}] Deleting person with ID ${person.id} from itslearning (if they exist), because they have no relevant personenkontexte!`, ); // We don't have any relevant personenkontexte for this person, so we delete it - const deleteError: Option = await this.itslearningPersonRepo.deletePerson(person.id); + const deleteError: Option = await this.itslearningPersonRepo.deletePerson( + person.id, + `${event.eventID}-DELETE`, + ); if (deleteError) { - return this.logger.error(`Could not delete person with ID ${person.id} from itslearning!`); + return this.logger.error( + `[EventID: ${event.eventID}] Could not delete person with ID ${person.id} from itslearning!`, + ); } } } diff --git a/src/modules/itslearning/itslearning.service.spec.ts b/src/modules/itslearning/itslearning.service.spec.ts index f0b49b227..8675e6c5d 100644 --- a/src/modules/itslearning/itslearning.service.spec.ts +++ b/src/modules/itslearning/itslearning.service.spec.ts @@ -58,6 +58,26 @@ describe('ItsLearningIMSESService', () => { }, ); }); + it('should include syncID if given', async () => { + const mockAction: DeepMocked> = createMock>(); + mockAction.buildRequest.mockReturnValueOnce({}); + mockAction.action = 'testAction'; + httpServiceMock.post.mockReturnValueOnce(of({} as AxiosResponse)); + const syncID: string = 'sync-id-test'; + + await sut.send(mockAction, syncID); + + expect(httpServiceMock.post).toHaveBeenCalledWith( + 'https://itslearning-test.example.com', + expect.stringContaining(syncID), + { + headers: { + 'Content-Type': 'text/xml;charset=UTF-8', + SOAPAction: `"testAction"`, + }, + }, + ); + }); it('should call parseResponse of action and return result', async () => { const mockAction: DeepMocked> = createMock>(); diff --git a/src/modules/itslearning/itslearning.service.ts b/src/modules/itslearning/itslearning.service.ts index d3c5c60e9..7a88e1f0d 100644 --- a/src/modules/itslearning/itslearning.service.ts +++ b/src/modules/itslearning/itslearning.service.ts @@ -10,6 +10,7 @@ import { ItsLearningConfig, ServerConfig } from '../../shared/config/index.js'; import { DomainError, ItsLearningError } from '../../shared/error/index.js'; import { IMSESAction } from './actions/base-action.js'; import { IMSESMassAction } from './actions/base-mass-action.js'; +import { IMS_MESS_BIND_SCHEMA } from './schemas.js'; @Injectable() export class ItsLearningIMSESService { @@ -34,9 +35,10 @@ export class ItsLearningIMSESService { public async send( action: IMSESAction | IMSESMassAction, + syncId?: string, ): Promise> { const body: object = action.buildRequest(); - const message: string = this.createMessage(body); + const message: string = this.createMessage(body, syncId); try { const response: AxiosResponse = await lastValueFrom( @@ -57,25 +59,29 @@ export class ItsLearningIMSESService { } } - private createMessage(body: object): string { + private createMessage(body: object, syncId?: string): string { return this.xmlBuilder.build({ 'soapenv:Envelope': { '@_xmlns:soapenv': 'http://schemas.xmlsoap.org/soap/envelope/', - 'soapenv:Header': this.createSecurityObject(), + 'soapenv:Header': this.createSecurityObject(syncId), 'soapenv:Body': body, }, }) as string; } - private createSecurityObject(): object { + private createSecurityObject(syncId?: string): object { const now: string = new Date().toISOString(); const nHash: Hash = createHash('sha1'); nHash.update(now + Math.random()); const nonce: string = nHash.digest('base64'); return { + 'ims:syncRequestHeaderInfo': syncId && { + '@_xmlns:ims': IMS_MESS_BIND_SCHEMA, + 'ims:messageIdentifier': syncId, + }, 'wsse:Security': { '@_xmlns:wsse': 'http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-secext-1.0.xsd', '@_xmlns:wsu': 'http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-utility-1.0.xsd', diff --git a/src/modules/itslearning/repo/itslearning-group.repo.spec.ts b/src/modules/itslearning/repo/itslearning-group.repo.spec.ts index 4f6a07abe..b2846fff9 100644 --- a/src/modules/itslearning/repo/itslearning-group.repo.spec.ts +++ b/src/modules/itslearning/repo/itslearning-group.repo.spec.ts @@ -46,11 +46,15 @@ describe('Itslearning Group Repo', () => { describe('readGroup', () => { it('should call the itslearning API', async () => { const organisationId: string = faker.string.uuid(); + const syncID: string = faker.string.uuid(); - await sut.readGroup(organisationId); + await sut.readGroup(organisationId, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ id: organisationId })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(ReadGroupAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ id: organisationId }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(ReadGroupAction), syncID); }); it('should return the result', async () => { @@ -93,11 +97,15 @@ describe('Itslearning Group Repo', () => { ok: true, value: undefined, }); // CreateGroupAction + const syncID: string = faker.string.uuid(); - await sut.createOrUpdateGroup(createParams); + await sut.createOrUpdateGroup(createParams, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ params: createParams })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreateGroupAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ params: createParams }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreateGroupAction), syncID); }); it('should not return error on success', async () => { @@ -150,11 +158,15 @@ describe('Itslearning Group Repo', () => { ok: true, value: undefined, }); // CreateGroupAction + const syncID: string = faker.string.uuid(); - await sut.createOrUpdateGroups(createParams); + await sut.createOrUpdateGroups(createParams, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ params: createParams })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreateGroupsAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ params: createParams }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreateGroupsAction), syncID); }); it('should not return error on success', async () => { @@ -204,11 +216,15 @@ describe('Itslearning Group Repo', () => { ok: true, value: undefined, }); // DeletePersonAction + const syncID: string = faker.string.uuid(); - await sut.deleteGroup(organisationId); + await sut.deleteGroup(organisationId, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ id: organisationId })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(DeleteGroupAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ id: organisationId }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(DeleteGroupAction), syncID); }); it('should not return error on success', async () => { diff --git a/src/modules/itslearning/repo/itslearning-group.repo.ts b/src/modules/itslearning/repo/itslearning-group.repo.ts index 9a69f075e..f9ee4bfff 100644 --- a/src/modules/itslearning/repo/itslearning-group.repo.ts +++ b/src/modules/itslearning/repo/itslearning-group.repo.ts @@ -13,9 +13,10 @@ import { ItsLearningIMSESService } from '../itslearning.service.js'; export class ItslearningGroupRepo { public constructor(private readonly itslearningService: ItsLearningIMSESService) {} - public async readGroup(id: OrganisationID): Promise> { + public async readGroup(id: OrganisationID, syncId?: string): Promise> { const groupResult: Result = await this.itslearningService.send( new ReadGroupAction(id), + syncId, ); if (!groupResult.ok) { @@ -25,10 +26,10 @@ export class ItslearningGroupRepo { return groupResult.value; } - public async createOrUpdateGroup(params: CreateGroupParams): Promise> { + public async createOrUpdateGroup(params: CreateGroupParams, syncId?: string): Promise> { const createAction: CreateGroupAction = new CreateGroupAction(params); - const createResult: Result = await this.itslearningService.send(createAction); + const createResult: Result = await this.itslearningService.send(createAction, syncId); if (!createResult.ok) { return createResult.error; @@ -37,10 +38,10 @@ export class ItslearningGroupRepo { return undefined; } - public async createOrUpdateGroups(params: CreateGroupParams[]): Promise> { + public async createOrUpdateGroups(params: CreateGroupParams[], syncId?: string): Promise> { const createAction: CreateGroupsAction = new CreateGroupsAction(params); - const createResult: Result = await this.itslearningService.send(createAction); + const createResult: Result = await this.itslearningService.send(createAction, syncId); if (!createResult.ok) { return createResult.error; @@ -49,8 +50,11 @@ export class ItslearningGroupRepo { return undefined; } - public async deleteGroup(id: OrganisationID): Promise> { - const deleteResult: Result = await this.itslearningService.send(new DeleteGroupAction(id)); + public async deleteGroup(id: OrganisationID, syncId?: string): Promise> { + const deleteResult: Result = await this.itslearningService.send( + new DeleteGroupAction(id), + syncId, + ); if (!deleteResult.ok) { return deleteResult.error; diff --git a/src/modules/itslearning/repo/itslearning-membership.repo.spec.ts b/src/modules/itslearning/repo/itslearning-membership.repo.spec.ts index c0bf749d7..16e8cfbb4 100644 --- a/src/modules/itslearning/repo/itslearning-membership.repo.spec.ts +++ b/src/modules/itslearning/repo/itslearning-membership.repo.spec.ts @@ -45,11 +45,15 @@ describe('Itslearning Person Repo', () => { describe('readMembershipsForPerson', () => { it('should call the itslearning API', async () => { const personId: string = faker.string.uuid(); + const syncID: string = faker.string.uuid(); - await sut.readMembershipsForPerson(personId); + await sut.readMembershipsForPerson(personId, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ personId })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(ReadMembershipsForPersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ personId }), syncID); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.any(ReadMembershipsForPersonAction), + syncID, + ); }); it('should return the result', async () => { @@ -83,11 +87,15 @@ describe('Itslearning Person Repo', () => { roleType: faker.helpers.enumValue(IMSESRoleType), }, ]; + const syncID: string = faker.string.uuid(); - await sut.createMemberships(memberships); + await sut.createMemberships(memberships, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ params: memberships })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreateMembershipsAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ params: memberships }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreateMembershipsAction), syncID); }); it('should not return error on success', async () => { @@ -133,11 +141,15 @@ describe('Itslearning Person Repo', () => { describe('removeMemberships', () => { it('should call the itslearning API', async () => { const membershipIDs: string[] = [faker.string.uuid()]; + const syncID: string = faker.string.uuid(); - await sut.removeMemberships(membershipIDs); + await sut.removeMemberships(membershipIDs, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ membershipIDs })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(DeleteMembershipsAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ membershipIDs }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(DeleteMembershipsAction), syncID); }); it('should not return error on success', async () => { @@ -171,11 +183,15 @@ describe('Itslearning Person Repo', () => { it('should read current memberships for person', async () => { const personId: string = faker.string.uuid(); itsLearningServiceMock.send.mockResolvedValueOnce({ ok: true, value: [] }); // Read Memberships + const syncID: string = faker.string.uuid(); - await sut.setMemberships(personId, []); + await sut.setMemberships(personId, [], syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ personId })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(ReadMembershipsForPersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ personId }), syncID); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.any(ReadMembershipsForPersonAction), + syncID, + ); }); it('should abort if memberships can not be read', async () => { diff --git a/src/modules/itslearning/repo/itslearning-membership.repo.ts b/src/modules/itslearning/repo/itslearning-membership.repo.ts index 51bcc2522..e50be729a 100644 --- a/src/modules/itslearning/repo/itslearning-membership.repo.ts +++ b/src/modules/itslearning/repo/itslearning-membership.repo.ts @@ -37,14 +37,23 @@ export class ItslearningMembershipRepo { this.ROOT_NAMES = [itslearningConfig.ROOT, itslearningConfig.ROOT_OEFFENTLICH, itslearningConfig.ROOT_ERSATZ]; } - public readMembershipsForPerson(personId: PersonID): Promise> { - return this.itslearningService.send(new ReadMembershipsForPersonAction(personId)); + public readMembershipsForPerson( + personId: PersonID, + syncId?: string, + ): Promise> { + return this.itslearningService.send(new ReadMembershipsForPersonAction(personId), syncId); } - public async createMemberships(memberships: CreateMembershipParams[]): Promise> { + public async createMemberships( + memberships: CreateMembershipParams[], + syncId?: string, + ): Promise> { const createMembershipsAction: CreateMembershipsAction = new CreateMembershipsAction(memberships); - const createResult: Result = await this.itslearningService.send(createMembershipsAction); + const createResult: Result = await this.itslearningService.send( + createMembershipsAction, + syncId, + ); if (!createResult.ok) { return createResult.error; @@ -53,9 +62,10 @@ export class ItslearningMembershipRepo { return undefined; } - public async removeMemberships(membershipIDs: string[]): Promise> { + public async removeMemberships(membershipIDs: string[], syncId?: string): Promise> { const deleteResult: Result = await this.itslearningService.send( new DeleteMembershipsAction(membershipIDs), + syncId, ); if (!deleteResult.ok) { @@ -68,14 +78,17 @@ export class ItslearningMembershipRepo { public async setMemberships( personId: PersonID, memberships: SetMembershipParams[], + syncId?: string, ): Promise> { const returnResult: SetMembershipsResult = { updated: 0, deleted: 0, }; - const currentMemberships: Result = - await this.readMembershipsForPerson(personId); + const currentMemberships: Result = await this.readMembershipsForPerson( + personId, + syncId, + ); if (!currentMemberships.ok) { return { @@ -111,6 +124,7 @@ export class ItslearningMembershipRepo { if (membershipsToBeRemoved.length > 0) { const deleteError: Option = await this.removeMemberships( membershipsToBeRemoved.map((mr: MembershipResponse) => mr.id), + syncId, ); if (deleteError) { @@ -134,7 +148,7 @@ export class ItslearningMembershipRepo { ); if (membershipsToCreateOrUpdate.length > 0) { - const createError: Option = await this.createMemberships(membershipsToCreateOrUpdate); + const createError: Option = await this.createMemberships(membershipsToCreateOrUpdate, syncId); if (createError) { this.logger.error( diff --git a/src/modules/itslearning/repo/itslearning-person.repo.spec.ts b/src/modules/itslearning/repo/itslearning-person.repo.spec.ts index 5243a6bb2..23e25b8ed 100644 --- a/src/modules/itslearning/repo/itslearning-person.repo.spec.ts +++ b/src/modules/itslearning/repo/itslearning-person.repo.spec.ts @@ -45,11 +45,12 @@ describe('Itslearning Person Repo', () => { describe('readPerson', () => { it('should call the itslearning API', async () => { const personId: string = faker.string.uuid(); + const syncID: string = faker.string.uuid(); - await sut.readPerson(personId); + await sut.readPerson(personId, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ id: personId })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(ReadPersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ id: personId }), syncID); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(ReadPersonAction), syncID); }); it('should return the result', async () => { @@ -101,11 +102,16 @@ describe('Itslearning Person Repo', () => { ok: true, value: undefined, }); // CreatePersonAction + const syncID: string = faker.string.uuid(); - await sut.updateEmail(personId, email); + await sut.updateEmail(personId, email, syncID); - expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith(1, expect.objectContaining({ id: personId })); - expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith(1, expect.any(ReadPersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ id: personId }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith(1, expect.any(ReadPersonAction), syncID); expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith( 2, expect.objectContaining({ @@ -118,8 +124,9 @@ describe('Itslearning Person Repo', () => { email, }, }), + syncID, ); - expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith(2, expect.any(CreatePersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenNthCalledWith(2, expect.any(CreatePersonAction), syncID); }); it('should not update email, if person was not found', async () => { @@ -150,11 +157,15 @@ describe('Itslearning Person Repo', () => { ok: true, value: undefined, }); // CreatePersonAction + const syncID: string = faker.string.uuid(); - await sut.createOrUpdatePerson(createParams); + await sut.createOrUpdatePerson(createParams, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ params: createParams })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreatePersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith( + expect.objectContaining({ params: createParams }), + syncID, + ); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(CreatePersonAction), syncID); }); it('should not return error on success', async () => { @@ -202,11 +213,12 @@ describe('Itslearning Person Repo', () => { ok: true, value: undefined, }); // DeletePersonAction + const syncID: string = faker.string.uuid(); - await sut.deletePerson(personId); + await sut.deletePerson(personId, syncID); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ id: personId })); - expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(DeletePersonAction)); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.objectContaining({ id: personId }), syncID); + expect(itsLearningServiceMock.send).toHaveBeenCalledWith(expect.any(DeletePersonAction), syncID); }); it('should not return error on success', async () => { diff --git a/src/modules/itslearning/repo/itslearning-person.repo.ts b/src/modules/itslearning/repo/itslearning-person.repo.ts index 219da1013..fc52458bd 100644 --- a/src/modules/itslearning/repo/itslearning-person.repo.ts +++ b/src/modules/itslearning/repo/itslearning-person.repo.ts @@ -11,9 +11,10 @@ import { ItsLearningIMSESService } from '../itslearning.service.js'; export class ItslearningPersonRepo { public constructor(private readonly itslearningService: ItsLearningIMSESService) {} - public async readPerson(id: PersonID): Promise> { + public async readPerson(id: PersonID, syncId?: string): Promise> { const personResult: Result = await this.itslearningService.send( new ReadPersonAction(id), + syncId, ); if (!personResult.ok) { @@ -23,10 +24,10 @@ export class ItslearningPersonRepo { return personResult.value; } - public async createOrUpdatePerson(params: CreatePersonParams): Promise> { + public async createOrUpdatePerson(params: CreatePersonParams, syncId?: string): Promise> { const createAction: CreatePersonAction = new CreatePersonAction(params); - const createResult: Result = await this.itslearningService.send(createAction); + const createResult: Result = await this.itslearningService.send(createAction, syncId); if (!createResult.ok) { return createResult.error; @@ -35,24 +36,30 @@ export class ItslearningPersonRepo { return undefined; } - public async updateEmail(personId: PersonID, email: string): Promise> { - const person: Option = await this.readPerson(personId); + public async updateEmail(personId: PersonID, email: string, syncId?: string): Promise> { + const person: Option = await this.readPerson(personId, syncId); // Person is not in itslearning, should not update the e-mail if (!person) return new ItsLearningError(`[updateEmail] person with ID ${personId} not found.`); - return this.createOrUpdatePerson({ - id: personId, - firstName: person.firstName, - lastName: person.lastName, - institutionRoleType: person.institutionRole, - username: person.username, - email, - }); + return this.createOrUpdatePerson( + { + id: personId, + firstName: person.firstName, + lastName: person.lastName, + institutionRoleType: person.institutionRole, + username: person.username, + email, + }, + syncId, + ); } - public async deletePerson(id: PersonID): Promise> { - const deleteResult: Result = await this.itslearningService.send(new DeletePersonAction(id)); + public async deletePerson(id: PersonID, syncId?: string): Promise> { + const deleteResult: Result = await this.itslearningService.send( + new DeletePersonAction(id), + syncId, + ); if (!deleteResult.ok) { return deleteResult.error; diff --git a/src/shared/events/base-event.ts b/src/shared/events/base-event.ts index 6b6e6658f..fd7e18504 100644 --- a/src/shared/events/base-event.ts +++ b/src/shared/events/base-event.ts @@ -1,3 +1,5 @@ +import { randomUUID } from 'crypto'; + declare const EVENT_MARKER: unique symbol; export abstract class BaseEvent { @@ -6,4 +8,7 @@ export abstract class BaseEvent { public readonly [EVENT_MARKER]!: never; public readonly createdAt: Date = new Date(); + + // For now generate random ID. In the future this should be replaced with the sessions correlation-ID or similar + public readonly eventID: string = randomUUID(); } From 7c955d4080d2d467758dc61fe5ab2b45e4796bb2 Mon Sep 17 00:00:00 2001 From: Cornelius <144817755+DPDS93CT@users.noreply.github.com> Date: Fri, 13 Dec 2024 06:09:40 +0100 Subject: [PATCH 7/8] SPSH-1583: Refaktorisierung OX-User-Blacklist (#829) * create OxUserBlacklistEntry, OxUserBlacklistRepo * use OxUserBlacklistRepo in UsernameGeneratorService * fix test-cases which use UsernameGenerator to provide OxUserBlacklistRepo * add test-cases for OxUserBlacklistRepo * fix imports in modules and test-files * rm unused import --- src/console/console.module.spec.ts | 5 +- src/console/console.module.ts | 2 - ...le-mocked-db-seed-repo.integration-spec.ts | 2 + .../db-seed.console.integration-spec.ts | 3 +- .../db-seed.service.integration-spec.ts | 3 +- .../email/persistence/email.repo.spec.ts | 2 + .../person/domain/ox-user-blacklist-entry.ts | 27 +++ ...name-generator.service.integration-spec.ts | 22 +- .../domain/username-generator.service.ts | 37 ++- .../ox-user-blacklist.repo.spec.ts | 216 ++++++++++++++++++ .../persistence/ox-user-blacklist.repo.ts | 104 +++++++++ src/modules/person/person.module.ts | 2 + ...onenkontext.controller.integration-spec.ts | 4 + ...m-personenkontext.repo.integration-spec.ts | 2 + ...m-personenkontext.repo.integration-spec.ts | 2 + ...kontext-specifications.integration-spec.ts | 4 + src/shared/types/ox-ids.types.ts | 3 + 17 files changed, 411 insertions(+), 29 deletions(-) create mode 100644 src/modules/person/domain/ox-user-blacklist-entry.ts create mode 100644 src/modules/person/persistence/ox-user-blacklist.repo.spec.ts create mode 100644 src/modules/person/persistence/ox-user-blacklist.repo.ts diff --git a/src/console/console.module.spec.ts b/src/console/console.module.spec.ts index 89818d0a7..c0f0bf291 100644 --- a/src/console/console.module.spec.ts +++ b/src/console/console.module.spec.ts @@ -2,13 +2,16 @@ import { Test, TestingModule } from '@nestjs/testing'; import { ConsoleModule } from './console.module.js'; import { DbConsole } from './db.console.js'; import { DbInitConsole } from './db-init.console.js'; +import { LoggingTestModule } from '../../test/utils/index.js'; +import { PersonModule } from '../modules/person/person.module.js'; +import { KeycloakAdministrationModule } from '../modules/keycloak-administration/keycloak-administration.module.js'; describe('ConsoleModule', () => { let module: TestingModule; beforeAll(async () => { module = await Test.createTestingModule({ - imports: [ConsoleModule], + imports: [PersonModule, KeycloakAdministrationModule, ConsoleModule, LoggingTestModule], }).compile(); }); diff --git a/src/console/console.module.ts b/src/console/console.module.ts index 512ed8491..6aba134c7 100644 --- a/src/console/console.module.ts +++ b/src/console/console.module.ts @@ -11,7 +11,6 @@ import { DbConsole } from './db.console.js'; import { DbInitConsole } from './db-init.console.js'; import { LoggerModule } from '../core/logging/logger.module.js'; import { KeycloakAdministrationModule } from '../modules/keycloak-administration/keycloak-administration.module.js'; -import { UsernameGeneratorService } from '../modules/person/domain/username-generator.service.js'; import { KeycloakConfigModule } from '../modules/keycloak-administration/keycloak-config.module.js'; import { OrganisationModule } from '../modules/organisation/organisation.module.js'; import { RolleModule } from '../modules/rolle/rolle.module.js'; @@ -93,7 +92,6 @@ import { KeycloakConsoleModule } from './keycloak/keycloak-console.module.js'; DbInitMigrationConsole, DbCreateMigrationConsole, DbApplyMigrationConsole, - UsernameGeneratorService, DbSeedDataGeneratorConsole, ], }) diff --git a/src/console/dbseed/db-seed.console-mocked-db-seed-repo.integration-spec.ts b/src/console/dbseed/db-seed.console-mocked-db-seed-repo.integration-spec.ts index 1d31758f2..6a9447b2f 100644 --- a/src/console/dbseed/db-seed.console-mocked-db-seed-repo.integration-spec.ts +++ b/src/console/dbseed/db-seed.console-mocked-db-seed-repo.integration-spec.ts @@ -25,6 +25,7 @@ import { DBiamPersonenkontextService } from '../../modules/personenkontext/domai import { DbSeedReferenceRepo } from './repo/db-seed-reference.repo.js'; import { PersonenKontextModule } from '../../modules/personenkontext/personenkontext.module.js'; import { LdapClient } from '../../core/ldap/domain/ldap-client.js'; +import { OxUserBlacklistRepo } from '../../modules/person/persistence/ox-user-blacklist.repo.js'; describe('DbSeedConsoleMockedDbSeedRepo', () => { let module: TestingModule; @@ -50,6 +51,7 @@ describe('DbSeedConsoleMockedDbSeedRepo', () => { providers: [ UsernameGeneratorService, DBiamPersonenkontextRepo, + OxUserBlacklistRepo, DbSeedConsole, DbSeedService, DBiamPersonenkontextService, diff --git a/src/console/dbseed/db-seed.console.integration-spec.ts b/src/console/dbseed/db-seed.console.integration-spec.ts index b449da2d8..67c12f9b6 100644 --- a/src/console/dbseed/db-seed.console.integration-spec.ts +++ b/src/console/dbseed/db-seed.console.integration-spec.ts @@ -19,6 +19,7 @@ import { RolleModule } from '../../modules/rolle/rolle.module.js'; import { ServiceProviderModule } from '../../modules/service-provider/service-provider.module.js'; import { DbSeedModule } from './db-seed.module.js'; import { PersonenKontextModule } from '../../modules/personenkontext/personenkontext.module.js'; +import { OxUserBlacklistRepo } from '../../modules/person/persistence/ox-user-blacklist.repo.js'; describe('DbSeedConsoleIntegration', () => { let module: TestingModule; @@ -41,7 +42,7 @@ describe('DbSeedConsoleIntegration', () => { ServiceProviderModule, PersonenKontextModule, ], - providers: [UsernameGeneratorService, DBiamPersonenkontextRepo], + providers: [UsernameGeneratorService, DBiamPersonenkontextRepo, OxUserBlacklistRepo], }) .overrideModule(KeycloakConfigModule) .useModule(KeycloakConfigTestModule.forRoot({ isKeycloakRequired: true })) diff --git a/src/console/dbseed/domain/db-seed.service.integration-spec.ts b/src/console/dbseed/domain/db-seed.service.integration-spec.ts index db936bad3..cf1ae8f90 100644 --- a/src/console/dbseed/domain/db-seed.service.integration-spec.ts +++ b/src/console/dbseed/domain/db-seed.service.integration-spec.ts @@ -21,6 +21,7 @@ import { PersonModule } from '../../../modules/person/person.module.js'; import { DbSeedModule } from '../db-seed.module.js'; import { PersonenKontextModule } from '../../../modules/personenkontext/personenkontext.module.js'; import { VornameForPersonWithTrailingSpaceError } from '../../../modules/person/domain/vorname-with-trailing-space.error.js'; +import { OxUserBlacklistRepo } from '../../../modules/person/persistence/ox-user-blacklist.repo.js'; describe('DbSeedServiceIntegration', () => { let module: TestingModule; @@ -42,7 +43,7 @@ describe('DbSeedServiceIntegration', () => { LoggingTestModule, PersonenKontextModule, ], - providers: [UsernameGeneratorService, DBiamPersonenkontextRepo], + providers: [UsernameGeneratorService, DBiamPersonenkontextRepo, OxUserBlacklistRepo], }) .overrideModule(KeycloakConfigModule) .useModule(KeycloakConfigTestModule.forRoot({ isKeycloakRequired: true })) diff --git a/src/modules/email/persistence/email.repo.spec.ts b/src/modules/email/persistence/email.repo.spec.ts index da0a88c36..c62b0e662 100644 --- a/src/modules/email/persistence/email.repo.spec.ts +++ b/src/modules/email/persistence/email.repo.spec.ts @@ -27,6 +27,7 @@ import { PersonAlreadyHasEnabledEmailAddressError } from '../error/person-alread import { UserLockRepository } from '../../keycloak-administration/repository/user-lock.repository.js'; import { PersonEmailResponse } from '../../person/api/person-email-response.js'; import { generatePassword } from '../../../shared/util/password-generator.js'; +import { OxUserBlacklistRepo } from '../../person/persistence/ox-user-blacklist.repo.js'; import { OrganisationID, PersonID } from '../../../shared/types/aggregate-ids.types.js'; describe('EmailRepo', () => { @@ -45,6 +46,7 @@ describe('EmailRepo', () => { imports: [ConfigTestModule, DatabaseTestModule.forRoot({ isDatabaseRequired: true })], providers: [ UsernameGeneratorService, + OxUserBlacklistRepo, EmailRepo, EmailFactory, PersonFactory, diff --git a/src/modules/person/domain/ox-user-blacklist-entry.ts b/src/modules/person/domain/ox-user-blacklist-entry.ts new file mode 100644 index 000000000..95c4adcbb --- /dev/null +++ b/src/modules/person/domain/ox-user-blacklist-entry.ts @@ -0,0 +1,27 @@ +import { OXEmail, OXUserName } from '../../../shared/types/ox-ids.types.js'; + +export class OxUserBlacklistEntry { + public constructor( + public id: Persisted, + public readonly createdAt: Persisted, + public readonly updatedAt: Persisted, + public email: OXEmail, + public name: string, + public username: OXUserName, + ) {} + + public static construct( + id: string, + createdAt: Date, + updatedAt: Date, + email: OXEmail, + name: string, + username: OXUserName, + ): OxUserBlacklistEntry { + return new OxUserBlacklistEntry(id, createdAt, updatedAt, email, name, username); + } + + public static createNew(email: OXEmail, name: string, username: OXUserName): OxUserBlacklistEntry { + return new OxUserBlacklistEntry(undefined, undefined, undefined, email, name, username); + } +} diff --git a/src/modules/person/domain/username-generator.service.integration-spec.ts b/src/modules/person/domain/username-generator.service.integration-spec.ts index 77e00397b..29d1046dc 100644 --- a/src/modules/person/domain/username-generator.service.integration-spec.ts +++ b/src/modules/person/domain/username-generator.service.integration-spec.ts @@ -18,11 +18,14 @@ import { ConfigTestModule, } from '../../../../test/utils/index.js'; import { MikroORM } from '@mikro-orm/core'; +import { OxUserBlacklistRepo } from '../persistence/ox-user-blacklist.repo.js'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; -describe('The UsernameGenerator Service', () => { +describe('UsernameGeneratorService', () => { let module: TestingModule; let service: UsernameGeneratorService; let kcUserService: DeepMocked; + let loggerMock: DeepMocked; let em: EntityManager; let orm: MikroORM; @@ -31,12 +34,21 @@ describe('The UsernameGenerator Service', () => { imports: [ConfigTestModule, DatabaseTestModule.forRoot({ isDatabaseRequired: true })], providers: [ UsernameGeneratorService, - { provide: KeycloakUserService, useValue: createMock() }, + OxUserBlacklistRepo, + { + provide: KeycloakUserService, + useValue: createMock(), + }, + { + provide: ClassLogger, + useValue: createMock(), + }, ], }).compile(); orm = module.get(MikroORM); service = module.get(UsernameGeneratorService); kcUserService = module.get(KeycloakUserService); + loggerMock = module.get(ClassLogger); em = module.get(EntityManager); await DatabaseTestModule.setupDatabase(orm); @@ -127,6 +139,7 @@ describe('The UsernameGenerator Service', () => { .mockResolvedValueOnce({ ok: true, value: createMock>() }) .mockResolvedValueOnce({ ok: false, error: new EntityNotFoundError('Not found') }); const generatedUsername: Result = await service.generateUsername('Max', 'Meyer'); + expect(loggerMock.info).toHaveBeenLastCalledWith(`Next Available Username Is:mmeyer1`); expect(generatedUsername).toEqual({ ok: true, value: 'mmeyer1' }); }); @@ -137,6 +150,7 @@ describe('The UsernameGenerator Service', () => { } else return Promise.resolve({ ok: false, error: new EntityNotFoundError('Not found') }); }); const generatedUsername: Result = await service.generateUsername('Max', 'Meyer'); + expect(loggerMock.info).toHaveBeenLastCalledWith(`Next Available Username Is:mmeyer2`); expect(generatedUsername).toEqual({ ok: true, value: 'mmeyer2' }); }); @@ -148,6 +162,7 @@ describe('The UsernameGenerator Service', () => { .mockResolvedValueOnce({ ok: false, error: new EntityNotFoundError('Not found') }) .mockResolvedValueOnce({ ok: true, value: createMock>() }); const generatedUsername: Result = await service.generateUsername('Renate', 'Bergmann'); + expect(loggerMock.info).toHaveBeenLastCalledWith(`Next Available Username Is:rbergmann3`); expect(generatedUsername).toEqual({ ok: true, value: 'rbergmann3' }); }); @@ -156,6 +171,7 @@ describe('The UsernameGenerator Service', () => { await expect(service.generateUsername('Maximilian', 'Mustermann')).rejects.toStrictEqual( new KeycloakClientError('Could not reach'), ); + expect(loggerMock.info).toHaveBeenCalledTimes(0); }); it('should return error if username can not be generated (cleaned names are of length 0)', async () => { @@ -199,6 +215,8 @@ describe('The UsernameGenerator Service', () => { const generatedUsername: Result = await service.generateUsername('Max', 'Meyer'); // Assert: The generated username should have the counter appended + expect(loggerMock.info).toHaveBeenLastCalledWith(`Next Available Username Is:mmeyer1`); + expect(generatedUsername).toEqual({ ok: true, value: 'mmeyer1' }); }); }); diff --git a/src/modules/person/domain/username-generator.service.ts b/src/modules/person/domain/username-generator.service.ts index 0a869038e..dbe0148b0 100644 --- a/src/modules/person/domain/username-generator.service.ts +++ b/src/modules/person/domain/username-generator.service.ts @@ -8,14 +8,16 @@ import { InvalidAttributeLengthError, } from '../../../shared/error/index.js'; import { isDIN91379A, toDIN91379SearchForm } from '../../../shared/util/din-91379-validation.js'; -import { OxUserBlacklistEntity } from '../persistence/ox-user-blacklist.entity.js'; -import { EntityManager } from '@mikro-orm/postgresql'; +import { OxUserBlacklistRepo } from '../persistence/ox-user-blacklist.repo.js'; +import { OxUserBlacklistEntry } from './ox-user-blacklist-entry.js'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; @Injectable() export class UsernameGeneratorService { public constructor( - private readonly em: EntityManager, + private readonly logger: ClassLogger, private kcUserService: KeycloakUserService, + private oxUserBlacklistRepo: OxUserBlacklistRepo, ) {} public async generateUsername(firstname: string, lastname: string): Promise> { @@ -80,22 +82,17 @@ export class UsernameGeneratorService { while (await this.usernameExists(calculatedUsername + counter)) { counter = counter + 1; } - /* eslint-disable no-await-in-loop */ - return calculatedUsername + counter; - } - - public async findByOxUsername(username: string): Promise { - const person: Option = await this.em.findOne(OxUserBlacklistEntity, { - username: username, - }); - if (person) { - return person; - } + this.logger.info(`Next Available Username Is:${calculatedUsername + counter}`); - return null; + return calculatedUsername + counter; } - public async usernameExists(username: string): Promise { + /** + * This method can throw errors e.g. if Keycloak search fails. + * @param username + * @private + */ + private async usernameExists(username: string): Promise { // Check Keycloak const searchResult: Result, DomainError> = await this.kcUserService.findOne({ username }); if (searchResult.ok) { @@ -105,12 +102,8 @@ export class UsernameGeneratorService { } // Check OX Blacklist for the username. If it exists then return true. - const oxUser: OxUserBlacklistEntity | null = await this.findByOxUsername(username); - - if (oxUser) { - return true; // Username exists in the blacklist - } + const oxUser: Option> = await this.oxUserBlacklistRepo.findByOxUsername(username); - return false; // Username is available + return !!oxUser; } } diff --git a/src/modules/person/persistence/ox-user-blacklist.repo.spec.ts b/src/modules/person/persistence/ox-user-blacklist.repo.spec.ts new file mode 100644 index 000000000..f6568268c --- /dev/null +++ b/src/modules/person/persistence/ox-user-blacklist.repo.spec.ts @@ -0,0 +1,216 @@ +import { faker } from '@faker-js/faker'; +import { Test, TestingModule } from '@nestjs/testing'; +import { + ConfigTestModule, + DatabaseTestModule, + DEFAULT_TIMEOUT_FOR_TESTCONTAINERS, +} from '../../../../test/utils/index.js'; +import { EntityManager, MikroORM } from '@mikro-orm/core'; +import { mapAggregateToData, OxUserBlacklistRepo } from './ox-user-blacklist.repo.js'; +import { OxUserBlacklistEntry } from '../domain/ox-user-blacklist-entry.js'; +import { OxUserBlacklistEntity } from './ox-user-blacklist.entity.js'; +import { OXEmail, OXUserName } from '../../../shared/types/ox-ids.types.js'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; +import { createMock } from '@golevelup/ts-jest'; +import { DomainError } from '../../../shared/error/domain.error.js'; +import { EntityNotFoundError } from '../../../shared/error/entity-not-found.error.js'; + +describe('OxUserBlacklistRepo', () => { + let module: TestingModule; + let sut: OxUserBlacklistRepo; + let orm: MikroORM; + let em: EntityManager; + + beforeAll(async () => { + module = await Test.createTestingModule({ + imports: [ConfigTestModule, DatabaseTestModule.forRoot({ isDatabaseRequired: true })], + providers: [ + OxUserBlacklistRepo, + { + provide: ClassLogger, + useValue: createMock(), + }, + ], + }).compile(); + sut = module.get(OxUserBlacklistRepo); + orm = module.get(MikroORM); + em = module.get(EntityManager); + + await DatabaseTestModule.setupDatabase(orm); + }, DEFAULT_TIMEOUT_FOR_TESTCONTAINERS); + + async function createEntity(email?: OXEmail, name?: string, username?: OXUserName): Promise { + const oxUserBlacklistEntity: OxUserBlacklistEntity = new OxUserBlacklistEntity(); + oxUserBlacklistEntity.email = email ?? faker.internet.email(); + oxUserBlacklistEntity.name = name ?? faker.person.lastName(); + oxUserBlacklistEntity.username = username ?? faker.internet.userName(); + await em.persistAndFlush(oxUserBlacklistEntity); + } + + async function createOxUserBlacklistEntry( + email?: OXEmail, + name?: string, + username?: OXUserName, + ): Promise { + const oxUserBlacklistEntry: OxUserBlacklistEntry = OxUserBlacklistEntry.createNew( + email ?? faker.internet.email(), + name ?? faker.person.lastName(), + username ?? faker.internet.userName(), + ); + const mappedOxUserBlacklistEntity: OxUserBlacklistEntity = em.create( + OxUserBlacklistEntity, + mapAggregateToData(oxUserBlacklistEntry), + ); + await em.persistAndFlush(mappedOxUserBlacklistEntity); + + return mappedOxUserBlacklistEntity; + } + + afterAll(async () => { + await orm.close(); + await module.close(); + }); + + beforeEach(async () => { + await DatabaseTestModule.clearDatabase(orm); + }); + + it('should be defined', () => { + expect(sut).toBeDefined(); + }); + + describe('findByEmail', () => { + describe('when entity can be found by email', () => { + it('should return OxUserBlacklistEntry', async () => { + const fakeEmail: OXEmail = faker.internet.email(); + await createEntity(fakeEmail); + + const findResult: Option> = await sut.findByEmail(fakeEmail); + + if (!findResult) throw Error(); + expect(findResult.email).toStrictEqual(fakeEmail); + }); + }); + + describe('when entity CANNOT be found by email', () => { + it('should return null', async () => { + const findResult: Option> = await sut.findByEmail(faker.internet.email()); + + expect(findResult).toBeNull(); + }); + }); + }); + + describe('findByOxUsername', () => { + describe('when entity can be found by username', () => { + it('should return OxUserBlacklistEntry', async () => { + const fakeUsername: OXUserName = faker.internet.userName(); + await createEntity(undefined, undefined, fakeUsername); + + const findResult: Option> = await sut.findByOxUsername(fakeUsername); + + if (!findResult) throw Error(); + expect(findResult.username).toStrictEqual(fakeUsername); + }); + }); + + describe('when entity CANNOT be found by username', () => { + it('should return null', async () => { + const findResult: Option> = await sut.findByOxUsername( + faker.internet.userName(), + ); + + expect(findResult).toBeNull(); + }); + }); + }); + + describe('save', () => { + beforeEach(() => { + jest.restoreAllMocks(); + }); + describe('when OxUserBlacklistEntry has an id and can be found', () => { + it('should call the update method and return the updated OxUserBlacklistEntry', async () => { + const existingEntity: OxUserBlacklistEntity = await createOxUserBlacklistEntry(); + const updatedEmail: OXEmail = faker.internet.email(); + const updatedName: string = faker.person.lastName(); + const updatedUsername: OXUserName = faker.internet.userName(); + + const updatedEntry: OxUserBlacklistEntry = OxUserBlacklistEntry.construct( + existingEntity.id, + existingEntity.createdAt, + existingEntity.updatedAt, + updatedEmail, + updatedName, + updatedUsername, + ); + + const result: OxUserBlacklistEntry | DomainError = await sut.save(updatedEntry); + if (result instanceof DomainError) throw Error(); + + const foundOxUserBlacklistEntity: Option = await em.findOne( + OxUserBlacklistEntity, + { + id: existingEntity.id, + }, + ); + if (!foundOxUserBlacklistEntity) throw Error(); + + expect(foundOxUserBlacklistEntity.id).toStrictEqual(existingEntity.id); + expect(foundOxUserBlacklistEntity.email).toStrictEqual(updatedEmail); + expect(foundOxUserBlacklistEntity.name).toStrictEqual(updatedName); + expect(foundOxUserBlacklistEntity.username).toStrictEqual(updatedUsername); + }); + }); + + describe('when OxUserBlacklistEntry has an id and CANNOT be found', () => { + it('should call the update method and return EntityNotFoundError', async () => { + const updatedEmail: OXEmail = faker.internet.email(); + const updatedName: string = faker.person.lastName(); + const updatedUsername: OXUserName = faker.internet.userName(); + + const updatedEntry: OxUserBlacklistEntry = OxUserBlacklistEntry.construct( + faker.string.uuid(), + faker.date.past(), + faker.date.recent(), + updatedEmail, + updatedName, + updatedUsername, + ); + + const result: OxUserBlacklistEntry | DomainError = await sut.save(updatedEntry); + if (result instanceof OxUserBlacklistEntry) throw Error(); + + expect(result).toStrictEqual(new EntityNotFoundError('OxUserBlacklistEntity')); + }); + }); + + describe('when OxUserBlacklistEntry does not have an id', () => { + it('should call the create method and return the created OxUserBlacklistEntry', async () => { + const newEmail: OXEmail = faker.internet.email(); + const newName: string = faker.person.lastName(); + const newUsername: OXUserName = faker.internet.userName(); + const newEntry: OxUserBlacklistEntry = OxUserBlacklistEntry.createNew( + newEmail, + newName, + newUsername, + ); + + const result: OxUserBlacklistEntry | DomainError = await sut.save(newEntry); + if (result instanceof DomainError) throw Error(); + + const foundOxUserBlacklistEntity: Option = await em.findOne( + OxUserBlacklistEntity, + { + email: newEmail, + }, + ); + if (!foundOxUserBlacklistEntity) throw Error(); + + expect(foundOxUserBlacklistEntity.email).toStrictEqual(newEmail); + expect(foundOxUserBlacklistEntity.name).toStrictEqual(newName); + expect(foundOxUserBlacklistEntity.username).toStrictEqual(newUsername); + }); + }); + }); +}); diff --git a/src/modules/person/persistence/ox-user-blacklist.repo.ts b/src/modules/person/persistence/ox-user-blacklist.repo.ts new file mode 100644 index 000000000..10df9c2e1 --- /dev/null +++ b/src/modules/person/persistence/ox-user-blacklist.repo.ts @@ -0,0 +1,104 @@ +import { EntityManager, RequiredEntityData } from '@mikro-orm/core'; +import { Injectable } from '@nestjs/common'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; +import { OxUserBlacklistEntry } from '../domain/ox-user-blacklist-entry.js'; +import { OxUserBlacklistEntity } from './ox-user-blacklist.entity.js'; +import { DomainError, EntityNotFoundError } from '../../../shared/error/index.js'; +import { OXEmail, OXUserName } from '../../../shared/types/ox-ids.types.js'; + +export function mapAggregateToData( + oxUserBlacklistEntry: OxUserBlacklistEntry, +): RequiredEntityData { + return { + // Don't assign createdAt and updatedAt, they are auto-generated! + id: oxUserBlacklistEntry.id, + email: oxUserBlacklistEntry.email, + name: oxUserBlacklistEntry.name, + username: oxUserBlacklistEntry.username, + }; +} + +function mapEntityToAggregate(entity: OxUserBlacklistEntity): OxUserBlacklistEntry { + return new OxUserBlacklistEntry( + entity.id, + entity.createdAt, + entity.updatedAt, + entity.email, + entity.name, + entity.username, + ); +} + +@Injectable() +export class OxUserBlacklistRepo { + public constructor( + private readonly em: EntityManager, + private readonly logger: ClassLogger, + ) {} + + public async findByEmail(email: OXEmail): Promise>> { + const oxUserBlacklistEntity: Option = await this.em.findOne(OxUserBlacklistEntity, { + email, + }); + if (oxUserBlacklistEntity) { + return mapEntityToAggregate(oxUserBlacklistEntity); + } + + return null; + } + + public async findByOxUsername(oxUsername: OXUserName): Promise>> { + const oxUserBlacklistEntity: Option = await this.em.findOne(OxUserBlacklistEntity, { + username: oxUsername, + }); + if (oxUserBlacklistEntity) { + return mapEntityToAggregate(oxUserBlacklistEntity); + } + + return null; + } + + /** + * Creates or updates OxUserBlacklistEntity in database. + * @param oxUserBlacklistEntry + */ + public async save( + oxUserBlacklistEntry: OxUserBlacklistEntry, + ): Promise | DomainError> { + if (oxUserBlacklistEntry.id) { + return this.update(oxUserBlacklistEntry); + } else { + return this.create(oxUserBlacklistEntry); + } + } + + private async create(oxUserBlacklistEntry: OxUserBlacklistEntry): Promise> { + const oxUserBlacklistEntity: OxUserBlacklistEntity = this.em.create( + OxUserBlacklistEntity, + mapAggregateToData(oxUserBlacklistEntry), + ); + await this.em.persistAndFlush(oxUserBlacklistEntity); + + return mapEntityToAggregate(oxUserBlacklistEntity); + } + + private async update( + oxUserBlacklistEntry: OxUserBlacklistEntry, + ): Promise | DomainError> { + const oxUserBlacklistEntity: Option = await this.em.findOne(OxUserBlacklistEntity, { + id: oxUserBlacklistEntry.id, + }); + + if (!oxUserBlacklistEntity) { + this.logger.error( + `Could Not Find OxUserBlacklistEntity, oxUserBlacklistEntryId:${oxUserBlacklistEntry.id}`, + ); + return new EntityNotFoundError('OxUserBlacklistEntity'); + } + + oxUserBlacklistEntity.assign(mapAggregateToData(oxUserBlacklistEntry)); + await this.em.persistAndFlush(oxUserBlacklistEntity); + + return mapEntityToAggregate(oxUserBlacklistEntity); + } +} diff --git a/src/modules/person/person.module.ts b/src/modules/person/person.module.ts index 45afb2505..5854b2cca 100644 --- a/src/modules/person/person.module.ts +++ b/src/modules/person/person.module.ts @@ -10,6 +10,7 @@ import { RolleFactory } from '../rolle/domain/rolle.factory.js'; import { ServiceProviderRepo } from '../service-provider/repo/service-provider.repo.js'; import { OrganisationRepository } from '../organisation/persistence/organisation.repository.js'; import { EventModule } from '../../core/eventbus/event.module.js'; +import { OxUserBlacklistRepo } from './persistence/ox-user-blacklist.repo.js'; @Module({ imports: [KeycloakAdministrationModule, LoggerModule.register(PersonModule.name), EventModule], providers: [ @@ -21,6 +22,7 @@ import { EventModule } from '../../core/eventbus/event.module.js'; OrganisationRepository, RolleFactory, ServiceProviderRepo, + OxUserBlacklistRepo, ], exports: [PersonService, PersonFactory, PersonRepository], }) diff --git a/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration-spec.ts b/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration-spec.ts index def47061c..7949c00fd 100644 --- a/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration-spec.ts +++ b/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration-spec.ts @@ -13,6 +13,7 @@ import { DatabaseTestModule, DoFactory, KeycloakConfigTestModule, + LoggingTestModule, MapperTestModule, } from '../../../../test/utils/index.js'; import { GlobalValidationPipe } from '../../../shared/validation/index.js'; @@ -36,6 +37,7 @@ import { PersonFactory } from '../../person/domain/person.factory.js'; import { UsernameGeneratorService } from '../../person/domain/username-generator.service.js'; import { PersonenkontextMigrationRuntype } from '../domain/personenkontext.enums.js'; import { generatePassword } from '../../../shared/util/password-generator.js'; +import { OxUserBlacklistRepo } from '../../person/persistence/ox-user-blacklist.repo.js'; describe('dbiam Personenkontext API', () => { let app: INestApplication; @@ -56,6 +58,7 @@ describe('dbiam Personenkontext API', () => { DatabaseTestModule.forRoot({ isDatabaseRequired: true }), PersonenKontextApiModule, KeycloakAdministrationModule, + LoggingTestModule, ], providers: [ { @@ -82,6 +85,7 @@ describe('dbiam Personenkontext API', () => { }, PersonFactory, UsernameGeneratorService, + OxUserBlacklistRepo, ], }) .overrideModule(KeycloakConfigModule) diff --git a/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.integration-spec.ts b/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.integration-spec.ts index 31431ec91..4de57a5eb 100644 --- a/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.integration-spec.ts +++ b/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.integration-spec.ts @@ -41,6 +41,7 @@ import { } from '../../../../test/utils/organisation-test-helper.js'; import { UserLockRepository } from '../../keycloak-administration/repository/user-lock.repository.js'; import { generatePassword } from '../../../shared/util/password-generator.js'; +import { OxUserBlacklistRepo } from '../../person/persistence/ox-user-blacklist.repo.js'; describe('dbiam Personenkontext Repo', () => { let module: TestingModule; @@ -94,6 +95,7 @@ describe('dbiam Personenkontext Repo', () => { PersonFactory, PersonRepository, UsernameGeneratorService, + OxUserBlacklistRepo, RolleFactory, RolleRepo, ServiceProviderRepo, diff --git a/src/modules/personenkontext/persistence/internal-dbiam-personenkontext.repo.integration-spec.ts b/src/modules/personenkontext/persistence/internal-dbiam-personenkontext.repo.integration-spec.ts index bbc54f564..9edc6cbcc 100644 --- a/src/modules/personenkontext/persistence/internal-dbiam-personenkontext.repo.integration-spec.ts +++ b/src/modules/personenkontext/persistence/internal-dbiam-personenkontext.repo.integration-spec.ts @@ -26,6 +26,7 @@ import { ServiceProviderRepo } from '../../service-provider/repo/service-provide import { DBiamPersonenkontextRepoInternal } from './internal-dbiam-personenkontext.repo.js'; import { UserLockRepository } from '../../keycloak-administration/repository/user-lock.repository.js'; import { generatePassword } from '../../../shared/util/password-generator.js'; +import { OxUserBlacklistRepo } from '../../person/persistence/ox-user-blacklist.repo.js'; describe('dbiam Personenkontext Repo', () => { let module: TestingModule; @@ -74,6 +75,7 @@ describe('dbiam Personenkontext Repo', () => { PersonFactory, PersonRepository, UsernameGeneratorService, + OxUserBlacklistRepo, RolleFactory, RolleRepo, ServiceProviderRepo, diff --git a/src/modules/personenkontext/specification/personenkontext-specifications.integration-spec.ts b/src/modules/personenkontext/specification/personenkontext-specifications.integration-spec.ts index 6f354d629..ab7e868ba 100644 --- a/src/modules/personenkontext/specification/personenkontext-specifications.integration-spec.ts +++ b/src/modules/personenkontext/specification/personenkontext-specifications.integration-spec.ts @@ -3,6 +3,7 @@ import { ConfigTestModule, DatabaseTestModule, KeycloakConfigTestModule, + LoggingTestModule, MapperTestModule, } from '../../../../test/utils/index.js'; import { RolleRepo } from '../../rolle/repo/rolle.repo.js'; @@ -27,6 +28,7 @@ import { OrganisationRepository } from '../../organisation/persistence/organisat import { EventService } from '../../../core/eventbus/index.js'; import { EmailRepo } from '../../email/persistence/email.repo.js'; import { Organisation } from '../../organisation/domain/organisation.js'; +import { OxUserBlacklistRepo } from '../../person/persistence/ox-user-blacklist.repo.js'; function createPersonenkontext( this: void, @@ -69,11 +71,13 @@ describe('PersonenkontextSpecifications Integration', () => { DatabaseTestModule.forRoot({ isDatabaseRequired: true }), KeycloakAdministrationModule, MapperTestModule, + LoggingTestModule, ], providers: [ PersonRepository, PersonFactory, UsernameGeneratorService, + OxUserBlacklistRepo, PersonenkontextFactory, { provide: EmailRepo, diff --git a/src/shared/types/ox-ids.types.ts b/src/shared/types/ox-ids.types.ts index 8b06b7158..947c0bc36 100644 --- a/src/shared/types/ox-ids.types.ts +++ b/src/shared/types/ox-ids.types.ts @@ -1,5 +1,8 @@ import { Flavor } from './flavor.types.js'; +declare const oxEmailSymbol: unique symbol; +export type OXEmail = Flavor; + declare const oxUserIdSymbol: unique symbol; export type OXUserID = Flavor; From c3e12c87c7fd055263979ed05327ef9081b5bfe3 Mon Sep 17 00:00:00 2001 From: Cornelius <144817755+DPDS93CT@users.noreply.github.com> Date: Mon, 16 Dec 2024 09:33:36 +0100 Subject: [PATCH 8/8] adjust config-loader to NOT convert values implicitly (#841) --- src/shared/config/config.loader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/config/config.loader.ts b/src/shared/config/config.loader.ts index d471bdc22..5fd3025b6 100644 --- a/src/shared/config/config.loader.ts +++ b/src/shared/config/config.loader.ts @@ -35,7 +35,7 @@ export function loadConfigFiles(): JsonConfig { merged = merge(json, env); } - const mergedConfig: JsonConfig = plainToInstance(JsonConfig, merged, { enableImplicitConversion: true }); + const mergedConfig: JsonConfig = plainToInstance(JsonConfig, merged, { enableImplicitConversion: false }); const errors: ValidationError[] = validateSync(mergedConfig, { skipMissingProperties: false,