Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SPSH-1501: Removal as member from OX-groups #802

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

SPSH-1501: Removal as member from OX-groups #802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
50f2aa5
SPSH-1506 (#788)
YoussefBouch Nov 28, 2024
caca051
Comment out enableForitslearning StepUpGuard (#796)
casparneumann-cap Nov 28, 2024
508e754
SPSH-1516: Implemented the removal of OX_ID in KC for lehrer when Ema…
phaelcg Nov 28, 2024
d79b1d7
SPSH-1520: reconfigure database health check (#797)
niklaskoopmann Nov 28, 2024
3c60e72
remove OxUser from all OxGroups on EmailAddressDisabledEvent
DPDS93CT Nov 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 0 additions & 159 deletions src/core/ldap/domain/ldap-event-handler.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,131 +473,6 @@ describe('LDAP Event Handler', () => {
expect(ldapClientServiceMock.createLehrer).toHaveBeenCalledTimes(0);
});

it('should call ldap client for every deleted personenkontext with correct role (if person has PK with rollenArt LEHR left)', async () => {
const event: PersonenkontextUpdatedEvent = new PersonenkontextUpdatedEvent(
{
id: faker.string.uuid(),
vorname: faker.person.firstName(),
familienname: faker.person.lastName(),
referrer: faker.internet.userName(),
},
[],
[
{
id: faker.string.uuid(),
orgaId: faker.string.uuid(),
rolle: RollenArt.LEHR,
rolleId: faker.string.uuid(),
orgaKennung: faker.string.numeric(7),
isItslearningOrga: false,
serviceProviderExternalSystems: [],
},
{
id: faker.string.uuid(),
orgaId: faker.string.uuid(),
rolle: RollenArt.EXTERN,
rolleId: faker.string.uuid(),
orgaKennung: faker.string.numeric(7),
isItslearningOrga: false,
serviceProviderExternalSystems: [],
},
],
[],
);

organisationRepositoryMock.findEmailDomainForOrganisation.mockResolvedValueOnce('schule-sh.de');

await ldapEventHandler.handlePersonenkontextUpdatedEvent(event);

expect(ldapClientServiceMock.deleteLehrer).toHaveBeenCalledTimes(1);
});

it('should NOT call ldap client for deleting person in LDAP when person still has at least one PK with rollenArt LEHR left', async () => {
const event: PersonenkontextUpdatedEvent = new PersonenkontextUpdatedEvent(
{
id: faker.string.uuid(),
vorname: faker.person.firstName(),
familienname: faker.person.lastName(),
referrer: faker.internet.userName(),
},
[],
[
{
id: faker.string.uuid(),
orgaId: faker.string.uuid(),
rolle: RollenArt.LEHR,
rolleId: faker.string.uuid(),
orgaKennung: faker.string.numeric(7),
isItslearningOrga: false,
serviceProviderExternalSystems: [],
},
],
[
{
id: faker.string.uuid(),
orgaId: faker.string.uuid(),
rolle: RollenArt.LEHR,
rolleId: faker.string.uuid(),
orgaKennung: faker.string.numeric(7),
isItslearningOrga: false,
serviceProviderExternalSystems: [],
},
],
);

organisationRepositoryMock.findEmailDomainForOrganisation.mockResolvedValueOnce('schule-sh.de');

await ldapEventHandler.handlePersonenkontextUpdatedEvent(event);

expect(ldapClientServiceMock.deleteLehrer).toHaveBeenCalledTimes(0);
expect(loggerMock.info).toHaveBeenLastCalledWith(
`Keep lehrer in LDAP, personId:${event.person.id}, because person keeps PK(s) with rollenArt LEHR`,
);
});

it('when organisation of deleted PK has no valid emailDomain should log error', async () => {
const removedOrgaId: string = faker.string.uuid();
const event: PersonenkontextUpdatedEvent = new PersonenkontextUpdatedEvent(
{
id: faker.string.uuid(),
vorname: faker.person.firstName(),
familienname: faker.person.lastName(),
referrer: faker.internet.userName(),
},
[],
[
{
id: faker.string.uuid(),
orgaId: removedOrgaId,
rolle: RollenArt.LEHR,
rolleId: faker.string.uuid(),
orgaKennung: faker.string.numeric(7),
isItslearningOrga: false,
serviceProviderExternalSystems: [],
},
{
id: faker.string.uuid(),
orgaId: faker.string.uuid(),
rolle: RollenArt.EXTERN,
rolleId: faker.string.uuid(),
orgaKennung: faker.string.numeric(7),
isItslearningOrga: false,
serviceProviderExternalSystems: [],
},
],
[],
);

organisationRepositoryMock.findEmailDomainForOrganisation.mockResolvedValueOnce(undefined);

await ldapEventHandler.handlePersonenkontextUpdatedEvent(event);

expect(loggerMock.error).toHaveBeenLastCalledWith(
`LdapClientService deleteLehrer NOT called, because organisation:${removedOrgaId} has no valid emailDomain`,
);
expect(ldapClientServiceMock.deleteLehrer).toHaveBeenCalledTimes(0);
});

describe('when ldap client fails', () => {
it('should execute without errors, if creation of lehrer fails', async () => {
const event: PersonenkontextUpdatedEvent = new PersonenkontextUpdatedEvent(
Expand Down Expand Up @@ -630,40 +505,6 @@ describe('LDAP Event Handler', () => {
expect(ldapClientServiceMock.createLehrer).toHaveBeenCalledTimes(1);
});
});

it('should execute without errors, if deletion of lehrer fails', async () => {
const event: PersonenkontextUpdatedEvent = new PersonenkontextUpdatedEvent(
{
id: faker.string.uuid(),
vorname: faker.person.firstName(),
familienname: faker.person.lastName(),
referrer: faker.internet.userName(),
},
[],
[
{
id: faker.string.uuid(),
orgaId: faker.string.uuid(),
rolle: RollenArt.LEHR,
rolleId: faker.string.uuid(),
orgaKennung: faker.string.numeric(7),
isItslearningOrga: false,
serviceProviderExternalSystems: [],
},
],
[],
);
ldapClientServiceMock.deleteLehrer.mockResolvedValueOnce({
ok: false,
error: new Error('Error'),
});

organisationRepositoryMock.findEmailDomainForOrganisation.mockResolvedValueOnce('schule-sh.de');

await ldapEventHandler.handlePersonenkontextUpdatedEvent(event);

expect(ldapClientServiceMock.deleteLehrer).toHaveBeenCalledTimes(1);
});
});

describe('handleEmailAddressGeneratedEvent', () => {
Expand Down
29 changes: 0 additions & 29 deletions src/core/ldap/domain/ldap-event-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,35 +152,6 @@ export class LdapEventHandler {
`Received PersonenkontextUpdatedEvent, personId:${event.person.id}, new personenkontexte: ${event.newKontexte.length}, deleted personenkontexte: ${event.removedKontexte.length}`,
);

if (!event.containsAnyCurrentPKWithRollenartLehr()) {
// Delete all removed personenkontexte if rollenart === LEHR
await Promise.allSettled(
event.removedKontexte
.filter((pk: PersonenkontextEventKontextData) => pk.rolle === RollenArt.LEHR)
.map(async (pk: PersonenkontextEventKontextData) => {
const emailDomain: Result<string> = await this.getEmailDomainForOrganisationId(pk.orgaId);
if (emailDomain.ok) {
this.logger.info(`Call LdapClientService because rollenArt is LEHR, pkId: ${pk.id}`);
const deletionResult: Result<PersonData> = await this.ldapClientService.deleteLehrer(
event.person,
emailDomain.value,
);
if (!deletionResult.ok) {
this.logger.error(deletionResult.error.message);
}
} else {
this.logger.error(
`LdapClientService deleteLehrer NOT called, because organisation:${pk.orgaId} has no valid emailDomain`,
);
}
}),
);
} else {
this.logger.info(
`Keep lehrer in LDAP, personId:${event.person.id}, because person keeps PK(s) with rollenArt LEHR`,
);
}

// Create personenkontexte if rollenart === LEHR
await Promise.allSettled(
event.newKontexte
Expand Down
82 changes: 61 additions & 21 deletions src/modules/email/domain/email-event-handler.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ describe('Email Event Handler', () => {
let serviceProviderRepoMock: DeepMocked<ServiceProviderRepo>;
let organisationRepositoryMock: DeepMocked<OrganisationRepository>;
let loggerMock: DeepMocked<ClassLogger>;
let personRepositoryMock: DeepMocked<PersonRepository>;

beforeAll(async () => {
const module: TestingModule = await Test.createTestingModule({
Expand Down Expand Up @@ -117,6 +118,7 @@ describe('Email Event Handler', () => {
dbiamPersonenkontextRepoMock = module.get(DBiamPersonenkontextRepo);
organisationRepositoryMock = module.get(OrganisationRepository);
loggerMock = module.get(ClassLogger);
personRepositoryMock = module.get(PersonRepository);

app = module.createNestApplication();
await app.init();
Expand Down Expand Up @@ -202,7 +204,7 @@ describe('Email Event Handler', () => {
rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap);
serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap);

emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce(undefined); //no existing email is found
emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found

const persistenceResult: EmailAddress<true> = getEmail();
emailRepoMock.save.mockResolvedValueOnce(persistenceResult); //mock: error during saving the entity
Expand Down Expand Up @@ -326,16 +328,16 @@ describe('Email Event Handler', () => {
serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap);

// eslint-disable-next-line @typescript-eslint/require-await
emailRepoMock.findEnabledByPerson.mockImplementationOnce(async (personId: PersonID) => {
return new EmailAddress<true>(
emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockImplementationOnce(async (personId: PersonID) => [
new EmailAddress<true>(
faker.string.uuid(),
faker.date.past(),
faker.date.recent(),
personId,
faker.internet.email(),
EmailAddressStatus.ENABLED,
);
});
),
]);

await emailEventHandler.handlePersonenkontextUpdatedEvent(event);

Expand All @@ -352,16 +354,16 @@ describe('Email Event Handler', () => {
serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap);

// eslint-disable-next-line @typescript-eslint/require-await
emailRepoMock.findEnabledByPerson.mockImplementationOnce(async (personId: PersonID) => {
return new EmailAddress<true>(
emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockImplementationOnce(async (personId: PersonID) => [
new EmailAddress<true>(
faker.string.uuid(),
faker.date.past(),
faker.date.recent(),
personId,
faker.internet.email(),
EmailAddressStatus.DISABLED,
);
});
),
]);

const persistedEmail: EmailAddress<true> = getEmail();
emailRepoMock.save.mockResolvedValueOnce(persistedEmail);
Expand All @@ -381,16 +383,16 @@ describe('Email Event Handler', () => {
serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap);

// eslint-disable-next-line @typescript-eslint/require-await
emailRepoMock.findEnabledByPerson.mockImplementationOnce(async (personId: PersonID) => {
return new EmailAddress<true>(
emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockImplementationOnce(async (personId: PersonID) => [
new EmailAddress<true>(
faker.string.uuid(),
faker.date.past(),
faker.date.recent(),
personId,
faker.internet.email(),
EmailAddressStatus.DISABLED,
);
});
),
]);

emailRepoMock.save.mockResolvedValueOnce(new EmailAddressNotFoundError(fakeEmailAddressString));

Expand All @@ -408,7 +410,7 @@ describe('Email Event Handler', () => {
rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap);
serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap);

emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce(undefined); //no existing email is found
emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found

const persistenceResult: EmailAddress<true> = getEmail();
emailRepoMock.save.mockResolvedValueOnce(persistenceResult); //mock: error during saving the entity
Expand Down Expand Up @@ -443,7 +445,7 @@ describe('Email Event Handler', () => {
rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap);
serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap);

emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(undefined); //no existing email is found
emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found

// eslint-disable-next-line @typescript-eslint/require-await
emailFactoryMock.createNew.mockImplementationOnce(async (personId: PersonID) => {
Expand Down Expand Up @@ -481,7 +483,7 @@ describe('Email Event Handler', () => {
rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap);
serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap);

emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(undefined); //no existing email is found
emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found

// eslint-disable-next-line @typescript-eslint/require-await
emailFactoryMock.createNew.mockImplementationOnce(async (personId: PersonID) => {
Expand Down Expand Up @@ -509,7 +511,7 @@ describe('Email Event Handler', () => {
rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap);
serviceProviderRepoMock.findByIds.mockResolvedValueOnce(spMap);

emailRepoMock.findEnabledByPerson.mockResolvedValueOnce(undefined); //no existing email is found
emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]); //no existing email is found

emailRepoMock.save.mockResolvedValueOnce(new EmailAddressNotFoundError(fakeEmailAddressString)); //mock: error during saving the entity

Expand Down Expand Up @@ -565,6 +567,7 @@ describe('Email Event Handler', () => {
dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte);
rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap);
serviceProviderRepoMock.findByIds.mockResolvedValueOnce(new Map<string, ServiceProvider<true>>());
personRepositoryMock.findById.mockResolvedValueOnce(createMock<Person<true>>());

const emailAddress: EmailAddress<true> = EmailAddress.construct(
faker.string.uuid(),
Expand Down Expand Up @@ -596,6 +599,46 @@ describe('Email Event Handler', () => {
});
});
});

describe(`When email is disabled and Person does not have an username, EmailAddressDisabledEvent is not published`, () => {
it('should log matching error', async () => {
dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte);
rolleRepoMock.findByIds.mockResolvedValueOnce(rolleMap);
serviceProviderRepoMock.findByIds.mockResolvedValueOnce(new Map<string, ServiceProvider<true>>());
personRepositoryMock.findById.mockResolvedValueOnce(createMock<Person<true>>({ referrer: undefined }));

const emailAddress: EmailAddress<true> = EmailAddress.construct(
faker.string.uuid(),
faker.date.past(),
faker.date.recent(),
fakePersonId,
faker.internet.email(),
EmailAddressStatus.DISABLED,
);

emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([
EmailAddress.construct(
faker.string.uuid(),
faker.date.past(),
faker.date.recent(),
fakePersonId,
faker.internet.email(),
EmailAddressStatus.ENABLED,
),
]);

emailRepoMock.save.mockResolvedValueOnce(emailAddress);
await emailEventHandler.handlePersonenkontextUpdatedEvent(event);

expect(loggerMock.info).toHaveBeenCalledWith(
expect.stringContaining('Existing email found for personId'),
);
expect(loggerMock.info).toHaveBeenCalledWith(`Disabled and saved address:${emailAddress.address}`);
expect(loggerMock.error).toHaveBeenCalledWith(
`Could not publish EmailAddressDisabledEvent, personId:${fakePersonId} has no username`,
);
});
});
});

describe('handlePersonenkontextCreatedMigrationEvent', () => {
Expand Down Expand Up @@ -640,10 +683,7 @@ describe('Email Event Handler', () => {
inputEmailAdress,
);

// eslint-disable-next-line @typescript-eslint/require-await
emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockImplementationOnce(async () => {
return undefined;
});
emailRepoMock.findByPersonSortedByUpdatedAtDesc.mockResolvedValueOnce([]);
emailRepoMock.save.mockResolvedValueOnce(createMock<EmailAddress<true>>());

await emailEventHandler.handlePersonenkontextCreatedMigrationEvent(event);
Expand Down
Loading
Loading