diff --git a/apps/server/src/modules/group/domain/group.spec.ts b/apps/server/src/modules/group/domain/group.spec.ts index b5ae8a03321..f2a22e1dc37 100644 --- a/apps/server/src/modules/group/domain/group.spec.ts +++ b/apps/server/src/modules/group/domain/group.spec.ts @@ -1,7 +1,7 @@ +import { RoleReference, UserDO } from '@shared/domain'; import { groupFactory, roleFactory, userDoFactory } from '@shared/testing'; import { ObjectId } from 'bson'; -import { RoleReference, UserDO } from '@shared/domain'; import { Group } from './group'; import { GroupUser } from './group-user'; @@ -135,4 +135,62 @@ describe('Group (Domain Object)', () => { }); }); }); + + describe('addUser', () => { + describe('when the user already exists in the group', () => { + const setup = () => { + const existingUser: GroupUser = new GroupUser({ + userId: new ObjectId().toHexString(), + roleId: new ObjectId().toHexString(), + }); + const group = groupFactory.build({ + users: [existingUser], + }); + + return { + group, + existingUser, + }; + }; + + it('should not add the user', () => { + const { group, existingUser } = setup(); + + group.addUser(existingUser); + + expect(group.users.length).toEqual(1); + }); + }); + + describe('when the user does not exist in the group', () => { + const setup = () => { + const newUser: GroupUser = new GroupUser({ + userId: new ObjectId().toHexString(), + roleId: new ObjectId().toHexString(), + }); + const group = groupFactory.build({ + users: [ + { + userId: new ObjectId().toHexString(), + roleId: new ObjectId().toHexString(), + }, + ], + }); + + return { + group, + newUser, + }; + }; + + it('should add the user', () => { + const { group, newUser } = setup(); + + group.addUser(newUser); + + expect(group.users).toContain(newUser); + expect(group.users.length).toEqual(2); + }); + }); + }); }); diff --git a/apps/server/src/modules/group/domain/group.ts b/apps/server/src/modules/group/domain/group.ts index 049043618ac..826bbd36b22 100644 --- a/apps/server/src/modules/group/domain/group.ts +++ b/apps/server/src/modules/group/domain/group.ts @@ -45,4 +45,10 @@ export class Group extends DomainObject { isEmpty(): boolean { return this.props.users.length === 0; } + + addUser(user: GroupUser): void { + if (!this.users.find((u) => u.userId === user.userId)) { + this.users.push(user); + } + } } diff --git a/apps/server/src/modules/provisioning/strategy/oidc/service/oidc-provisioning.service.spec.ts b/apps/server/src/modules/provisioning/strategy/oidc/service/oidc-provisioning.service.spec.ts index c4b27cac34b..7e6e05c7b2e 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/service/oidc-provisioning.service.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/service/oidc-provisioning.service.spec.ts @@ -661,7 +661,7 @@ describe('OidcProvisioningService', () => { describe('when provision group', () => { const setup = () => { - const group: Group = groupFactory.build(); + const group: Group = groupFactory.build({ users: [] }); groupService.findByExternalSource.mockResolvedValue(group); const school: LegacySchoolDo = legacySchoolDoFactory.build({ id: 'schoolId' }); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/service/oidc-provisioning.service.ts b/apps/server/src/modules/provisioning/strategy/oidc/service/oidc-provisioning.service.ts index 0aef3fdecb9..31b2d1ab8f3 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/service/oidc-provisioning.service.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/service/oidc-provisioning.service.ts @@ -119,10 +119,6 @@ export class OidcProvisioningService { } async provisionExternalGroup(externalGroup: ExternalGroupDto, systemId: EntityId): Promise { - if (externalGroup.users.length === 0) { - return; - } - const existingGroup: Group | null = await this.groupService.findByExternalSource( externalGroup.externalId, systemId @@ -145,6 +141,10 @@ export class OidcProvisioningService { const users: GroupUser[] = await this.getFilteredGroupUsers(externalGroup, systemId); + if (!users.length) { + return; + } + const group: Group = new Group({ id: existingGroup ? existingGroup.id : new ObjectId().toHexString(), name: externalGroup.name, @@ -156,8 +156,9 @@ export class OidcProvisioningService { organizationId, validFrom: externalGroup.from, validUntil: externalGroup.until, - users, + users: existingGroup ? existingGroup.users : [], }); + users.forEach((user: GroupUser) => group.addUser(user)); await this.groupService.save(group); } @@ -168,7 +169,7 @@ export class OidcProvisioningService { const user: UserDO | null = await this.userService.findByExternalId(externalGroupUser.externalUserId, systemId); const roles: RoleDto[] = await this.roleService.findByNames([externalGroupUser.roleName]); - if (!user || !user.id || roles.length !== 1 || !roles[0].id) { + if (!user?.id || roles.length !== 1 || !roles[0].id) { this.logger.info(new UserForGroupNotFoundLoggable(externalGroupUser)); return null; } diff --git a/apps/server/src/modules/provisioning/strategy/sanis/sanis-response.mapper.spec.ts b/apps/server/src/modules/provisioning/strategy/sanis/sanis-response.mapper.spec.ts index 902e2e850f2..c52e654155b 100644 --- a/apps/server/src/modules/provisioning/strategy/sanis/sanis-response.mapper.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/sanis/sanis-response.mapper.spec.ts @@ -171,10 +171,6 @@ describe('SanisResponseMapper', () => { until: group.gruppe.laufzeit.bis, externalId: group.gruppe.id, users: [ - { - externalUserId: group.sonstige_gruppenzugehoerige![0].ktid, - roleName: RoleName.STUDENT, - }, { externalUserId: personenkontext.id, roleName: RoleName.TEACHER, @@ -206,9 +202,7 @@ describe('SanisResponseMapper', () => { describe('when a group role mapping is missing', () => { const setup = () => { const { sanisResponse } = setupSanisResponse(); - sanisResponse.personenkontexte[0].gruppen![0]!.sonstige_gruppenzugehoerige![0].rollen = [ - SanisGroupRole.SCHOOL_SUPPORT, - ]; + sanisResponse.personenkontexte[0].gruppen![0]!.gruppenzugehoerigkeit.rollen = [SanisGroupRole.SCHOOL_SUPPORT]; return { sanisResponse, @@ -220,7 +214,7 @@ describe('SanisResponseMapper', () => { const result: ExternalGroupDto[] | undefined = mapper.mapToExternalGroupDtos(sanisResponse); - expect(result![0].users).toHaveLength(1); + expect(result![0].users).toHaveLength(0); }); }); diff --git a/apps/server/src/modules/provisioning/strategy/sanis/sanis-response.mapper.ts b/apps/server/src/modules/provisioning/strategy/sanis/sanis-response.mapper.ts index 34a7ff4029c..e11e57e9068 100644 --- a/apps/server/src/modules/provisioning/strategy/sanis/sanis-response.mapper.ts +++ b/apps/server/src/modules/provisioning/strategy/sanis/sanis-response.mapper.ts @@ -66,7 +66,7 @@ export class SanisResponseMapper { } mapToExternalGroupDtos(source: SanisResponse): ExternalGroupDto[] | undefined { - const groups: SanisGruppenResponse[] | undefined = source.personenkontexte[0].gruppen; + const groups: SanisGruppenResponse[] | undefined = source.personenkontexte[0]?.gruppen; if (!groups) { return undefined; @@ -81,12 +81,11 @@ export class SanisResponseMapper { } const sanisGroupUsers: SanisSonstigeGruppenzugehoerigeResponse[] = [ - ...(group.sonstige_gruppenzugehoerige ?? []), { ktid: source.personenkontexte[0].id, rollen: group.gruppenzugehoerigkeit.rollen, }, - ].sort((a, b) => a.ktid.localeCompare(b.ktid)); + ].filter((sanisGroupUser) => sanisGroupUser.ktid && sanisGroupUser.rollen); const gruppenzugehoerigkeiten: ExternalGroupUserDto[] = sanisGroupUsers .map((relation): ExternalGroupUserDto | null => this.mapToExternalGroupUser(relation))