From 3c60e728cddded76ff863eab9b506a9dafeeda55 Mon Sep 17 00:00:00 2001 From: DPDS93CT Date: Sat, 30 Nov 2024 11:39:37 +0100 Subject: [PATCH 1/2] remove OxUser from all OxGroups on EmailAddressDisabledEvent --- .../group/remove-member-from-group.action.ts | 3 +- .../ox/domain/ox-event-handler.spec.ts | 134 +++++++++++++++++- src/modules/ox/domain/ox-event-handler.ts | 92 +++++++++++- src/shared/config/ox.config.ts | 6 +- 4 files changed, 225 insertions(+), 10 deletions(-) diff --git a/src/modules/ox/actions/group/remove-member-from-group.action.ts b/src/modules/ox/actions/group/remove-member-from-group.action.ts index 202b70ee6..416887605 100644 --- a/src/modules/ox/actions/group/remove-member-from-group.action.ts +++ b/src/modules/ox/actions/group/remove-member-from-group.action.ts @@ -1,12 +1,11 @@ import { DomainError } from '../../../../shared/error/domain.error.js'; import { NS2_SCHEMA, NS6_SCHEMA, TNS_SCHEMA } from '../../schemas.js'; import { OxBaseAction, OXRequestStatus } from '../ox-base-action.js'; -import { AddMemberToGroupResponseBody } from './add-member-to-group.action.js'; import { GroupMemberParams } from './ox-group.types.js'; export type RemoveMemberFromGroupResponse = { status: OXRequestStatus; - data: AddMemberToGroupResponseBody; + data: RemoveMemberFromGroupResponseBody; }; export type RemoveMemberFromGroupResponseBody = { diff --git a/src/modules/ox/domain/ox-event-handler.spec.ts b/src/modules/ox/domain/ox-event-handler.spec.ts index bc17c1831..3693329f5 100644 --- a/src/modules/ox/domain/ox-event-handler.spec.ts +++ b/src/modules/ox/domain/ox-event-handler.spec.ts @@ -24,6 +24,7 @@ import { EntityCouldNotBeCreated } from '../../../shared/error/index.js'; import { OXGroupID, OXUserID } from '../../../shared/types/ox-ids.types.js'; import { ListGroupsAction } from '../actions/group/list-groups.action.js'; import { EmailAddressAlreadyExistsEvent } from '../../../shared/events/email-address-already-exists.event.js'; +import { EmailAddressDisabledEvent } from '../../../shared/events/email-address-disabled.event.js'; describe('OxEventHandler', () => { let module: TestingModule; @@ -167,7 +168,7 @@ describe('OxEventHandler', () => { expect(oxServiceMock.send).toHaveBeenCalledWith(expect.any(CreateUserAction)); expect(loggerMock.error).toHaveBeenCalledWith( - `Could Not Create OxGroup with name:lehrer-${fakeDstNr}, displayName:Lehrer of ${fakeDstNr}`, + `Could Not Create OxGroup with name:lehrer-${fakeDstNr}, displayName:lehrer-${fakeDstNr}`, ); expect(eventServiceMock.publish).toHaveBeenCalledTimes(0); }); @@ -217,7 +218,7 @@ describe('OxEventHandler', () => { value: { groups: [ { - displayname: `Lehrer of ${fakeDstNr}`, + displayname: `lehrer-${fakeDstNr}`, id: 'id', name: `lehrer-${fakeDstNr}`, memberIds: [], @@ -370,13 +371,13 @@ describe('OxEventHandler', () => { value: { groups: [ { - displayname: `Lehrer of ${fakeDstNr}`, + displayname: `lehrer-${fakeDstNr}`, id: 'id', name: `lehrer-${fakeDstNr}`, memberIds: [], }, { - displayname: `Lehrer of ${fakeDstNr}`, + displayname: `lehrer-${fakeDstNr}`, id: 'id', name: `lehrer-${fakeDstNr}-`, memberIds: [], @@ -1048,4 +1049,129 @@ describe('OxEventHandler', () => { }); }); }); + + describe('handleEmailAddressDisabledEvent', () => { + let personId: PersonID; + let username: string; + let oxUserId: OXUserID; + let event: EmailAddressDisabledEvent; + let person: Person; + + beforeEach(() => { + jest.resetAllMocks(); + personId = faker.string.uuid(); + username = faker.internet.userName(); + oxUserId = faker.string.numeric(); + event = new EmailAddressDisabledEvent(personId, username); + person = createMock>({ + email: faker.internet.email(), + referrer: username, + oxUserId: oxUserId, + }); + }); + + describe('when handler is disabled', () => { + it('should log and skip processing when not enabled', async () => { + sut.ENABLED = false; + await sut.handleEmailAddressDisabledEvent(event); + + expect(loggerMock.info).toHaveBeenCalledWith('Not enabled, ignoring event'); + }); + }); + + describe('when person is not found', () => { + it('should log error if person does not exist', async () => { + personRepositoryMock.findById.mockResolvedValueOnce(undefined); + + await sut.handleEmailAddressDisabledEvent(event); + + expect(loggerMock.error).toHaveBeenCalledWith(`Could Not Find Person For personId:${event.personId}`); + }); + }); + + describe('when person is found BUT has NO oxUserId', () => { + it('should log error if person does not have a oxUserId', async () => { + personRepositoryMock.findById.mockResolvedValueOnce(createMock>({ oxUserId: undefined })); + + await sut.handleEmailAddressDisabledEvent(event); + + expect(loggerMock.error).toHaveBeenCalledWith( + `Could Not Remove Person From OxGroups, No OxUserId For personId:${event.personId}`, + ); + }); + }); + + describe('when listing groups for user fails', () => { + it('should log error', async () => { + personRepositoryMock.findById.mockResolvedValueOnce(person); + + //mock listing groups results in error + oxServiceMock.send.mockResolvedValueOnce({ + ok: false, + error: new OxError(), + }); + + await sut.handleEmailAddressDisabledEvent(event); + + expect(loggerMock.error).toHaveBeenCalledWith( + `Retrieving OxGroups For OxUser Failed, personId:${event.personId}`, + ); + }); + }); + + describe('when removing user from groups fails for at least one group', () => { + it('should log error', async () => { + personRepositoryMock.findById.mockResolvedValueOnce(person); + + //mock listing groups + oxServiceMock.send.mockResolvedValueOnce({ + ok: true, + value: { + groups: [ + { + displayname: 'group1-displayname', + id: 'group1-id', + name: 'group1-name', + memberIds: [oxUserId], + }, + { + displayname: 'group2-displayname', + id: 'group2-id', + name: 'group2-name', + memberIds: [oxUserId], + }, + ], + }, + }); + + //mock removing member from group-1 succeeds + oxServiceMock.send.mockResolvedValueOnce({ + ok: true, + value: { + status: { + code: 'success', + }, + data: 'body', + }, + }); + + //mock removing member from group-2 results in error + oxServiceMock.send.mockResolvedValueOnce({ + ok: false, + error: new OxError(), + }); + + await sut.handleEmailAddressDisabledEvent(event); + + expect(loggerMock.info).toHaveBeenCalledWith( + `Successfully Removed OxUser From OxGroup, oxUserId:${oxUserId}, oxGroupId:group1-id`, + ); + + expect(loggerMock.error).toHaveBeenCalledWith( + `Could Not Remove OxUser From OxGroup, oxUserId:${oxUserId}, oxGroupId:group2-id`, + ); + //expect(loggerMock.error).toHaveBeenCalledWith(`Failed To Removed OxUser From All Non-Standard OxGroups, personId:${event.personId}, oxUserId:${oxUserId}`); + }); + }); + }); }); diff --git a/src/modules/ox/domain/ox-event-handler.ts b/src/modules/ox/domain/ox-event-handler.ts index a47d78138..7f5271da0 100644 --- a/src/modules/ox/domain/ox-event-handler.ts +++ b/src/modules/ox/domain/ox-event-handler.ts @@ -23,7 +23,7 @@ import { UserIdParams, UserNameParams } from '../actions/user/ox-user.types.js'; import { EmailRepo } from '../../email/persistence/email.repo.js'; import { EmailAddress, EmailAddressStatus } from '../../email/domain/email-address.js'; import { AddMemberToGroupAction, AddMemberToGroupResponse } from '../actions/group/add-member-to-group.action.js'; -import { GroupMemberParams } from '../actions/group/ox-group.types.js'; +import { GroupMemberParams, OXGroup } from '../actions/group/ox-group.types.js'; import { CreateGroupAction, CreateGroupParams, CreateGroupResponse } from '../actions/group/create-group.action.js'; import { OxGroupNotFoundError } from '../error/ox-group-not-found.error.js'; import { ListGroupsAction, ListGroupsParams, ListGroupsResponse } from '../actions/group/list-groups.action.js'; @@ -33,6 +33,16 @@ import { ChangeByModuleAccessParams, } from '../actions/user/change-by-module-access.action.js'; import { EmailAddressAlreadyExistsEvent } from '../../../shared/events/email-address-already-exists.event.js'; +import { EmailAddressDisabledEvent } from '../../../shared/events/email-address-disabled.event.js'; +import { + ListGroupsForUserAction, + ListGroupsForUserParams, + ListGroupsForUserResponse, +} from '../actions/group/list-groups-for-user.action.js'; +import { + RemoveMemberFromGroupAction, + RemoveMemberFromGroupResponse, +} from '../actions/group/remove-member-from-group.action.js'; @Injectable() export class OxEventHandler { @@ -48,7 +58,7 @@ export class OxEventHandler { private static readonly LEHRER_OX_GROUP_NAME_PREFIX: string = 'lehrer-'; - private static readonly LEHRER_OX_GROUP_DISPLAY_NAME_PREFIX: string = 'Lehrer of '; + private static readonly LEHRER_OX_GROUP_DISPLAY_NAME_PREFIX: string = 'lehrer-'; public constructor( private readonly logger: ClassLogger, @@ -161,6 +171,37 @@ export class OxEventHandler { } } + @EventHandler(EmailAddressDisabledEvent) + public async handleEmailAddressDisabledEvent(event: EmailAddressDisabledEvent): Promise { + this.logger.info(`Received EmailAddressDisabledEvent, personId:${event.personId}, username:${event.username}`); + + if (!this.ENABLED) { + return this.logger.info('Not enabled, ignoring event'); + } + + const person: Option> = await this.personRepository.findById(event.personId); + if (!person) return this.logger.error(`Could Not Find Person For personId:${event.personId}`); + if (!person.oxUserId) + return this.logger.error( + `Could Not Remove Person From OxGroups, No OxUserId For personId:${event.personId}`, + ); + + const listGroupsForUserResponse: Result = await this.getOxGroupsForOxUserId( + person.oxUserId, + ); + if (!listGroupsForUserResponse.ok) { + return this.logger.error(`Retrieving OxGroups For OxUser Failed, personId:${event.personId}`); + } + //Removal from Standard-Group is possible even when user is member of other OxGroups + const oxGroups: OXGroup[] = listGroupsForUserResponse.value.groups; + const oxUserId: OXUserID = person.oxUserId; + + //logging of results is done in removeOxUserFromOxGroup + await Promise.allSettled( + oxGroups.map((oxGroup: OXGroup) => this.removeOxUserFromOxGroup(oxGroup.id, oxUserId)), + ); + } + private async getMostRecentRequestedEmailAddress(personId: PersonID): Promise>> { const requestedEmailAddresses: Option[]> = await this.emailRepo.findByPersonSortedByUpdatedAtDesc(personId, EmailAddressStatus.REQUESTED); @@ -269,8 +310,53 @@ export class OxEventHandler { if (!result.ok) { this.logger.error(`Could Not Add OxUser To OxGroup, oxUserId:${oxUserId}, oxGroupId:${oxGroupId}`); + } else { + this.logger.info(`Successfully Added OxUser To OxGroup, oxUserId:${oxUserId}, oxGroupId:${oxGroupId}`); + } + + return result; + } + + private async getOxGroupsForOxUserId(oxUserId: OXUserID): Promise> { + const params: ListGroupsForUserParams = { + contextId: this.contextID, + userId: oxUserId, + login: this.authUser, + password: this.authPassword, + }; + + const action: ListGroupsForUserAction = new ListGroupsForUserAction(params); + const result: Result = await this.oxService.send(action); + + if (!result.ok) { + this.logger.error(`Could Not Retrieve OxGroups For OxUser, oxUserId:${oxUserId}`); + } else { + this.logger.info(`Successfully Retrieved OxGroups For OxUser, oxUserId:${oxUserId}`); + } + + return result; + } + + private async removeOxUserFromOxGroup( + oxGroupId: OXGroupID, + oxUserId: OXUserID, + ): Promise> { + const params: GroupMemberParams = { + contextId: this.contextID, + groupId: oxGroupId, + memberId: oxUserId, + login: this.authUser, + password: this.authPassword, + }; + + const action: RemoveMemberFromGroupAction = new RemoveMemberFromGroupAction(params); + const result: Result = await this.oxService.send(action); + + if (!result.ok) { + this.logger.error(`Could Not Remove OxUser From OxGroup, oxUserId:${oxUserId}, oxGroupId:${oxGroupId}`); + } else { + this.logger.info(`Successfully Removed OxUser From OxGroup, oxUserId:${oxUserId}, oxGroupId:${oxGroupId}`); } - this.logger.info(`Successfully Added OxUser To OxGroup, oxUserId:${oxUserId}, oxGroupId:${oxGroupId}`); return result; } diff --git a/src/shared/config/ox.config.ts b/src/shared/config/ox.config.ts index d8ada0442..72c7a3f1e 100644 --- a/src/shared/config/ox.config.ts +++ b/src/shared/config/ox.config.ts @@ -1,4 +1,4 @@ -import { IsBooleanString, IsNumberString, IsString } from 'class-validator'; +import { IsBooleanString, IsNumberString, IsOptional, IsString } from 'class-validator'; export class OxConfig { @IsBooleanString() @@ -13,6 +13,10 @@ export class OxConfig { @IsString() public readonly CONTEXT_NAME!: string; + @IsNumberString() + @IsOptional() + public readonly STANDARD_GROUP_ID?: string; + @IsString() public readonly USERNAME!: string; From 80021dcaccabc3bac2386310c6190fe2009f03f0 Mon Sep 17 00:00:00 2001 From: DPDS93CT Date: Mon, 2 Dec 2024 12:03:02 +0100 Subject: [PATCH 2/2] adjust removing OxUser from OxGroups to be awaited --- src/modules/email/domain/email-event-handler.spec.ts | 4 ++-- src/modules/email/domain/email-event-handler.ts | 4 ++-- src/modules/ox/domain/ox-event-handler.ts | 10 ++++++---- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/modules/email/domain/email-event-handler.spec.ts b/src/modules/email/domain/email-event-handler.spec.ts index b015cb517..d9137c1c1 100644 --- a/src/modules/email/domain/email-event-handler.spec.ts +++ b/src/modules/email/domain/email-event-handler.spec.ts @@ -1107,8 +1107,8 @@ describe('Email Event Handler', () => { await emailEventHandler.handleOxMetadataInKeycloakChangedEvent(event); - expect(loggerMock.error).toHaveBeenLastCalledWith( - `Cannot find requested email-address for person with personId:${event.personId}, enabling not possible`, + expect(loggerMock.info).toHaveBeenLastCalledWith( + `Cannot find requested email-address for person with personId:${event.personId}, enabling not necessary`, ); }); }); diff --git a/src/modules/email/domain/email-event-handler.ts b/src/modules/email/domain/email-event-handler.ts index 71f3f461d..c7da91b33 100644 --- a/src/modules/email/domain/email-event-handler.ts +++ b/src/modules/email/domain/email-event-handler.ts @@ -247,8 +247,8 @@ export class EmailEventHandler { const email: Option> = await this.emailRepo.findRequestedByPerson(event.personId); if (!email) { - return this.logger.error( - `Cannot find requested email-address for person with personId:${event.personId}, enabling not possible`, + return this.logger.info( + `Cannot find requested email-address for person with personId:${event.personId}, enabling not necessary`, ); } diff --git a/src/modules/ox/domain/ox-event-handler.ts b/src/modules/ox/domain/ox-event-handler.ts index 7f5271da0..f1ca05314 100644 --- a/src/modules/ox/domain/ox-event-handler.ts +++ b/src/modules/ox/domain/ox-event-handler.ts @@ -196,10 +196,12 @@ export class OxEventHandler { const oxGroups: OXGroup[] = listGroupsForUserResponse.value.groups; const oxUserId: OXUserID = person.oxUserId; - //logging of results is done in removeOxUserFromOxGroup - await Promise.allSettled( - oxGroups.map((oxGroup: OXGroup) => this.removeOxUserFromOxGroup(oxGroup.id, oxUserId)), - ); + // The sent Ox-request should be awaited explicitly to avoid failures due to async execution in OX-Database (SQL-exceptions) + /* eslint-disable no-await-in-loop */ + for (const oxGroup of oxGroups) { + //logging of results is done in removeOxUserFromOxGroup + await this.removeOxUserFromOxGroup(oxGroup.id, oxUserId); + } } private async getMostRecentRequestedEmailAddress(personId: PersonID): Promise>> {