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..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": [], @@ -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..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": [], @@ -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/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/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/email/domain/email-event-handler.spec.ts b/src/modules/email/domain/email-event-handler.spec.ts index d9137c1c1..bf85cc012 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; @@ -148,7 +148,7 @@ describe('Email Event Handler', () => { }); } - 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('Email Event Handler', () => { }); }); }); + + 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('Email Event Handler', () => { 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('Email Event Handler', () => { ), ]); + //mock person with referrer is found + personRepositoryMock.findById.mockResolvedValueOnce(createMock>()); + await emailEventHandler.handlePersonenkontextUpdatedEvent(event); expect(loggerMock.info).toHaveBeenCalledWith( @@ -347,12 +371,50 @@ describe('Email Event Handler', () => { }); }); + //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('Email Event Handler', () => { 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('Email Event Handler', () => { 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('Email Event Handler', () => { 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('Email Event Handler', () => { 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('Email Event Handler', () => { 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('Email Event Handler', () => { 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('Email Event Handler', () => { 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('Email Event Handler', () => { 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); @@ -1129,10 +1220,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 +1243,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 +1262,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..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; @@ -253,9 +254,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,12 +267,41 @@ 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}`, + ); } } + 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>(); @@ -391,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`); @@ -406,6 +442,7 @@ export class EmailEventHandler { this.eventService.publish( new EmailAddressGeneratedEvent( personId, + personReferrer.value, persistenceResult.id, persistenceResult.address, persistenceResult.enabled, @@ -444,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); @@ -458,6 +499,7 @@ export class EmailEventHandler { this.eventService.publish( new EmailAddressGeneratedEvent( personId, + personReferrer.value, persistenceResult.id, persistenceResult.address, persistenceResult.enabled, @@ -476,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); @@ -491,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/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 0513f6411..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, @@ -546,7 +550,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}`, ); }); @@ -599,7 +603,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 +724,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`, @@ -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(), @@ -828,7 +833,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 +883,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 +905,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 +921,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..9183ad28e 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]; } @@ -508,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; @@ -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, 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 c0c565335..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; } @@ -301,8 +305,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) @@ -536,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 4ab102d5d..f17e42ba2 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(); }); }); @@ -1335,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 2a44931d3..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; @@ -357,7 +371,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 +399,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/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/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', } 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;