From f407956da33506f39c40d16ca3db9b71facb220e Mon Sep 17 00:00:00 2001 From: Caspar Neumann Date: Wed, 10 Jul 2024 16:17:24 +0200 Subject: [PATCH 1/5] Refactor Person-Permissions --- .../domain/person-permissions.spec.ts | 52 ++++++------------- .../domain/person-permissions.ts | 30 ++++++++--- ...biam-person.controller.integration-spec.ts | 8 +-- .../person/domain/person.service.spec.ts | 4 +- src/modules/person/domain/person.service.ts | 2 +- ...onenkontext.controller.integration.spec.ts | 12 ++--- .../domain/personenkontext.factory.ts | 4 ++ .../domain/personenkontext.spec.ts | 8 +-- .../personenkontext/domain/personenkontext.ts | 10 +++- .../persistence/dbiam-personenkontext.repo.ts | 36 ++++++++++++- 10 files changed, 103 insertions(+), 63 deletions(-) diff --git a/src/modules/authentication/domain/person-permissions.spec.ts b/src/modules/authentication/domain/person-permissions.spec.ts index 1f97b0de7..825a1b686 100644 --- a/src/modules/authentication/domain/person-permissions.spec.ts +++ b/src/modules/authentication/domain/person-permissions.spec.ts @@ -242,7 +242,7 @@ describe('PersonPermissions', () => { }); }); - describe('hasSystemrechtAtOrganisation', () => { + describe('hasSystemrechteAtOrganisation', () => { it('should return true if person has the recht', async () => { const person: Person = Person.construct( faker.string.uuid(), @@ -255,16 +255,7 @@ describe('PersonPermissions', () => { undefined, faker.string.uuid(), ); - const personenkontexte: Personenkontext[] = [ - personenkontextFactory.construct('1', faker.date.past(), faker.date.recent(), '1', '1', '1'), - ]; - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce( - new Map([['1', createMock>({ hasSystemRecht: () => true })]]), - ); - organisationRepoMock.findChildOrgasForIds.mockResolvedValueOnce([ - createMock>({ id: '2' }), - ]); + dbiamPersonenkontextRepoMock.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); const personPermissions: PersonPermissions = new PersonPermissions( dbiamPersonenkontextRepoMock, @@ -272,16 +263,17 @@ describe('PersonPermissions', () => { rolleRepoMock, person, ); - const result: boolean = await personPermissions.hasSystemrechtAtOrganisation('2', [ + const result: boolean = await personPermissions.hasSystemrechteAtOrganisation('2', [ RollenSystemRecht.PERSONEN_VERWALTEN, ]); - expect(result).toBe(true); + expect(result).toBeTruthy(); + expect(dbiamPersonenkontextRepoMock.hasSystemrechtAtOrganisation).toHaveBeenCalledTimes(1); }); }); - describe('hasSystemrechtAtOrganisation', () => { - it('should return true if person has the recht at the root', async () => { + describe('hasSystemrechteAtRootOrganisation', () => { + it('should return true if person has the recht', async () => { const person: Person = Person.construct( faker.string.uuid(), faker.date.past(), @@ -293,23 +285,7 @@ describe('PersonPermissions', () => { undefined, faker.string.uuid(), ); - const personenkontexte: Personenkontext[] = [ - personenkontextFactory.construct( - '1', - faker.date.past(), - faker.date.recent(), - '1', - organisationRepoMock.ROOT_ORGANISATION_ID, - '1', - ), - ]; - dbiamPersonenkontextRepoMock.findByPerson.mockResolvedValueOnce(personenkontexte); - rolleRepoMock.findByIds.mockResolvedValueOnce( - new Map([['1', createMock>({ hasSystemRecht: () => true })]]), - ); - organisationRepoMock.findChildOrgasForIds.mockResolvedValueOnce([ - createMock>({ id: organisationRepoMock.ROOT_ORGANISATION_ID }), - ]); + dbiamPersonenkontextRepoMock.hasSystemrechtAtOrganisation.mockResolvedValue(true); const personPermissions: PersonPermissions = new PersonPermissions( dbiamPersonenkontextRepoMock, @@ -317,11 +293,13 @@ describe('PersonPermissions', () => { rolleRepoMock, person, ); - const result: boolean = await personPermissions.hasSystemrechtAtRootOrganisation([ + const result: boolean = await personPermissions.hasSystemrechteAtRootOrganisation([ RollenSystemRecht.PERSONEN_VERWALTEN, + RollenSystemRecht.KLASSEN_VERWALTEN, ]); - expect(result).toBe(true); + expect(result).toBeTruthy(); + expect(dbiamPersonenkontextRepoMock.hasSystemrechtAtOrganisation).toHaveBeenCalledTimes(2); }); }); @@ -344,7 +322,7 @@ describe('PersonPermissions', () => { rolleRepoMock, person, ); - jest.spyOn(personPermissions, 'hasSystemrechtAtRootOrganisation').mockResolvedValueOnce(true); + jest.spyOn(personPermissions, 'hasSystemrechteAtRootOrganisation').mockResolvedValueOnce(true); const result: boolean = await personPermissions.canModifyPerson('2'); @@ -373,7 +351,7 @@ describe('PersonPermissions', () => { rolleRepoMock, person, ); - jest.spyOn(personPermissions, 'hasSystemrechtAtRootOrganisation').mockResolvedValueOnce(false); + jest.spyOn(personPermissions, 'hasSystemrechteAtRootOrganisation').mockResolvedValueOnce(false); const result: boolean = await personPermissions.canModifyPerson('2'); @@ -402,7 +380,7 @@ describe('PersonPermissions', () => { rolleRepoMock, person, ); - jest.spyOn(personPermissions, 'hasSystemrechtAtRootOrganisation').mockResolvedValueOnce(false); + jest.spyOn(personPermissions, 'hasSystemrechteAtRootOrganisation').mockResolvedValueOnce(false); const result: boolean = await personPermissions.canModifyPerson('2'); diff --git a/src/modules/authentication/domain/person-permissions.ts b/src/modules/authentication/domain/person-permissions.ts index 22c8422f7..5464da69b 100644 --- a/src/modules/authentication/domain/person-permissions.ts +++ b/src/modules/authentication/domain/person-permissions.ts @@ -62,6 +62,9 @@ export class PersonPermissions { }); } + /** + * @deprecated Inefficient + */ public async getOrgIdsWithSystemrecht( systemrechte: RollenSystemRecht[], withChildren: boolean = false, @@ -91,24 +94,37 @@ export class PersonPermissions { return Array.from(organisationIDs); } - public async hasSystemrechtAtOrganisation( + public async hasSystemrechteAtOrganisation( organisationId: OrganisationID, systemrechte: RollenSystemRecht[], ): Promise { - const orgsWithRecht: OrganisationID[] = await this.getOrgIdsWithSystemrecht(systemrechte, true); + const checks: Promise[] = systemrechte.map( + (systemrecht: RollenSystemRecht): Promise => + this.hasSystemrechtAtOrganisation(organisationId, systemrecht), + ); + const results: boolean[] = await Promise.all(checks); - return orgsWithRecht.includes(organisationId); + return results.every((result: boolean): boolean => result); } - public async hasSystemrechtAtRootOrganisation(systemrechte: RollenSystemRecht[]): Promise { - const orgsWithRecht: OrganisationID[] = await this.getOrgIdsWithSystemrecht(systemrechte, true); + public async hasSystemrechteAtRootOrganisation(systemrechte: RollenSystemRecht[]): Promise { + return this.hasSystemrechteAtOrganisation(this.organisationRepo.ROOT_ORGANISATION_ID, systemrechte); + } - return orgsWithRecht.includes(this.organisationRepo.ROOT_ORGANISATION_ID); + public async hasSystemrechtAtOrganisation( + organisationId: OrganisationID, + systemrecht: RollenSystemRecht, + ): Promise { + return this.personenkontextRepo.hasSystemrechtAtOrganisation( + this.cachedPersonFields.id, + organisationId, + systemrecht, + ); } public async canModifyPerson(personId: PersonID): Promise { { - const hasModifyRechtAtRoot: boolean = await this.hasSystemrechtAtRootOrganisation([ + const hasModifyRechtAtRoot: boolean = await this.hasSystemrechteAtRootOrganisation([ RollenSystemRecht.PERSONEN_VERWALTEN, ]); diff --git a/src/modules/person/api/dbiam-person.controller.integration-spec.ts b/src/modules/person/api/dbiam-person.controller.integration-spec.ts index b636f14cd..4debdbcb9 100644 --- a/src/modules/person/api/dbiam-person.controller.integration-spec.ts +++ b/src/modules/person/api/dbiam-person.controller.integration-spec.ts @@ -109,7 +109,7 @@ describe('dbiam Person API', () => { const personpermissions: DeepMocked = createMock(); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); personpermissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce([organisation.id]); - personpermissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); + personpermissions.hasSystemrechteAtOrganisation.mockResolvedValueOnce(true); const response: Response = await request(app.getHttpServer() as App) .post('/dbiam/personen') @@ -132,7 +132,7 @@ describe('dbiam Person API', () => { ); const permissions: DeepMocked = createMock(); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValueOnce(permissions); - permissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); + permissions.hasSystemrechteAtOrganisation.mockResolvedValueOnce(true); permissions.canModifyPerson.mockResolvedValueOnce(true); const response: Response = await request(app.getHttpServer() as App) @@ -159,7 +159,7 @@ describe('dbiam Person API', () => { ); const personpermissions: DeepMocked = createMock(); - personpermissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); + personpermissions.hasSystemrechteAtOrganisation.mockResolvedValueOnce(true); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); personpermissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce([organisation.id]); @@ -191,7 +191,7 @@ describe('dbiam Person API', () => { ); const personpermissions: DeepMocked = createMock(); - personpermissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(false); + personpermissions.hasSystemrechteAtOrganisation.mockResolvedValueOnce(false); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); const response: Response = await request(app.getHttpServer() as App) diff --git a/src/modules/person/domain/person.service.spec.ts b/src/modules/person/domain/person.service.spec.ts index 8050a693b..7df503346 100644 --- a/src/modules/person/domain/person.service.spec.ts +++ b/src/modules/person/domain/person.service.spec.ts @@ -250,7 +250,7 @@ describe('sut', () => { organisationRepositoryMock.findById.mockResolvedValueOnce( createMock>({ typ: OrganisationsTyp.LAND }), ); - personpermissionsMock.hasSystemrechtAtOrganisation.mockResolvedValueOnce(false); + personpermissionsMock.hasSystemrechteAtOrganisation.mockResolvedValueOnce(false); const result: PersonPersonenkontext | DomainError = await sut.createPersonWithPersonenkontext( personpermissionsMock, @@ -270,7 +270,7 @@ describe('sut', () => { organisationRepositoryMock.findById.mockResolvedValueOnce( createMock>({ typ: OrganisationsTyp.LAND }), ); - personpermissionsMock.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); + personpermissionsMock.hasSystemrechteAtOrganisation.mockResolvedValueOnce(true); personRepositoryMock.create.mockResolvedValueOnce( new KeycloakClientError('Username or email already exists'), ); diff --git a/src/modules/person/domain/person.service.ts b/src/modules/person/domain/person.service.ts index 92697a7df..49fc52e47 100644 --- a/src/modules/person/domain/person.service.ts +++ b/src/modules/person/domain/person.service.ts @@ -145,7 +145,7 @@ export class PersonService { organisationId: string, ): Promise> { // Check if logged in person has permission - const hasPermissionAtOrga: boolean = await permissions.hasSystemrechtAtOrganisation(organisationId, [ + const hasPermissionAtOrga: boolean = await permissions.hasSystemrechteAtOrganisation(organisationId, [ RollenSystemRecht.PERSONEN_VERWALTEN, ]); diff --git a/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration.spec.ts b/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration.spec.ts index 80fb79226..a034ce657 100644 --- a/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration.spec.ts +++ b/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration.spec.ts @@ -183,7 +183,7 @@ describe('dbiam Personenkontext API', () => { const personpermissions: DeepMocked = createMock(); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); personpermissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce([]); - personpermissions.hasSystemrechtAtRootOrganisation.mockResolvedValueOnce(false); + personpermissions.hasSystemrechteAtRootOrganisation.mockResolvedValueOnce(false); const response: Response = await request(app.getHttpServer() as App) .get(`/dbiam/personenkontext/${faker.string.uuid()}`) @@ -215,7 +215,7 @@ describe('dbiam Personenkontext API', () => { const personpermissions: DeepMocked = createMock(); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); personpermissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce([organisation.id]); - personpermissions.hasSystemrechtAtRootOrganisation.mockResolvedValueOnce(true); + personpermissions.hasSystemrechteAtRootOrganisation.mockResolvedValueOnce(true); const response: Response = await request(app.getHttpServer() as App) .post('/dbiam/personenkontext') @@ -247,7 +247,7 @@ describe('dbiam Personenkontext API', () => { const personpermissions: DeepMocked = createMock(); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); personpermissions.getOrgIdsWithSystemrecht.mockResolvedValueOnce([schule.id, klasse.id]); - personpermissions.hasSystemrechtAtRootOrganisation.mockResolvedValueOnce(true); + personpermissions.hasSystemrechteAtRootOrganisation.mockResolvedValueOnce(true); const response: Response = await request(app.getHttpServer() as App) .post('/dbiam/personenkontext') @@ -276,7 +276,7 @@ describe('dbiam Personenkontext API', () => { ); const permissions: DeepMocked = createMock(); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValueOnce(permissions); - permissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); + permissions.hasSystemrechteAtOrganisation.mockResolvedValueOnce(true); permissions.canModifyPerson.mockResolvedValueOnce(true); const response: Response = await request(app.getHttpServer() as App) @@ -402,7 +402,7 @@ describe('dbiam Personenkontext API', () => { const personpermissions: DeepMocked = createMock(); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); - personpermissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(false); + personpermissions.hasSystemrechteAtOrganisation.mockResolvedValueOnce(false); const response: Response = await request(app.getHttpServer() as App) .post('/dbiam/personenkontext') @@ -433,7 +433,7 @@ describe('dbiam Personenkontext API', () => { const personpermissions: DeepMocked = createMock(); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); - personpermissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(false); + personpermissions.hasSystemrechteAtOrganisation.mockResolvedValueOnce(false); const response: Response = await request(app.getHttpServer() as App) .post('/dbiam/personenkontext') diff --git a/src/modules/personenkontext/domain/personenkontext.factory.ts b/src/modules/personenkontext/domain/personenkontext.factory.ts index de1cb05a7..31cf10b79 100644 --- a/src/modules/personenkontext/domain/personenkontext.factory.ts +++ b/src/modules/personenkontext/domain/personenkontext.factory.ts @@ -4,10 +4,12 @@ import { Personenkontext } from './personenkontext.js'; import { RolleRepo } from '../../rolle/repo/rolle.repo.js'; import { PersonRepository } from '../../person/persistence/person.repository.js'; import { OrganisationRepository } from '../../organisation/persistence/organisation.repository.js'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; @Injectable() export class PersonenkontextFactory { public constructor( + private readonly logger: ClassLogger, private readonly personRepo: PersonRepository, private readonly organisationRepo: OrganisationRepository, private readonly rolleRepo: RolleRepo, @@ -22,6 +24,7 @@ export class PersonenkontextFactory { rolleId: RolleID, ): Personenkontext { return Personenkontext.construct( + this.logger, this.personRepo, this.organisationRepo, this.rolleRepo, @@ -36,6 +39,7 @@ export class PersonenkontextFactory { public createNew(personId: PersonID, organisationId: OrganisationID, rolleId: RolleID): Personenkontext { return Personenkontext.createNew( + this.logger, this.personRepo, this.organisationRepo, this.rolleRepo, diff --git a/src/modules/personenkontext/domain/personenkontext.spec.ts b/src/modules/personenkontext/domain/personenkontext.spec.ts index 616fd63ca..a1ddaf94a 100644 --- a/src/modules/personenkontext/domain/personenkontext.spec.ts +++ b/src/modules/personenkontext/domain/personenkontext.spec.ts @@ -176,7 +176,7 @@ describe('Personenkontext aggregate', () => { describe('checkPermissions', () => { it('should return MissingPermissionsError, if logged in user is not authorized at organisation', async () => { const permissions: DeepMocked = createMock(); - permissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(false); // Check orga permissions + permissions.hasSystemrechteAtOrganisation.mockResolvedValueOnce(false); // Check orga permissions const personenkontext: Personenkontext = personenkontextFactory.createNew( faker.string.uuid(), @@ -188,14 +188,14 @@ describe('Personenkontext aggregate', () => { new MissingPermissionsError('Unauthorized to manage persons at the organisation'), ); - expect(permissions.hasSystemrechtAtOrganisation).toHaveBeenCalledWith(personenkontext.organisationId, [ + expect(permissions.hasSystemrechteAtOrganisation).toHaveBeenCalledWith(personenkontext.organisationId, [ RollenSystemRecht.PERSONEN_VERWALTEN, ]); }); it('should return MissingPermissionsError, if target person can not be modified by logged in user', async () => { const permissions: DeepMocked = createMock(); - permissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); // Check orga permissions + permissions.hasSystemrechteAtOrganisation.mockResolvedValueOnce(true); // Check orga permissions const rolleMock: DeepMocked> = createMock>(); rolleMock.canBeAssignedToOrga.mockResolvedValueOnce(true); // Check rolle<->orga validity rolleRepoMock.findById.mockResolvedValueOnce(rolleMock); @@ -214,7 +214,7 @@ describe('Personenkontext aggregate', () => { it('should not return an error, if kontext is valid', async () => { const permissions: DeepMocked = createMock(); - permissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); // Check orga permissions + permissions.hasSystemrechteAtOrganisation.mockResolvedValueOnce(true); // Check orga permissions const rolleMock: DeepMocked> = createMock>(); rolleMock.canBeAssignedToOrga.mockResolvedValueOnce(true); // Check rolle<->orga validity rolleRepoMock.findById.mockResolvedValueOnce(rolleMock); diff --git a/src/modules/personenkontext/domain/personenkontext.ts b/src/modules/personenkontext/domain/personenkontext.ts index b828b3ae5..fbfbd121c 100644 --- a/src/modules/personenkontext/domain/personenkontext.ts +++ b/src/modules/personenkontext/domain/personenkontext.ts @@ -11,6 +11,7 @@ import { Rolle } from '../../rolle/domain/rolle.js'; import { RolleRepo } from '../../rolle/repo/rolle.repo.js'; import { OrganisationMatchesRollenart } from '../specification/organisation-matches-rollenart.js'; import { OrganisationMatchesRollenartError } from '../specification/error/organisation-matches-rollenart.error.js'; +import { ClassLogger } from '../../../core/logging/class-logger.js'; export type PersonenkontextPartial = Pick< Personenkontext, @@ -39,9 +40,11 @@ export class Personenkontext { public readonly personId: PersonID, public readonly organisationId: OrganisationID, public readonly rolleId: RolleID, + public readonly logger: ClassLogger, ) {} public static construct( + logger: ClassLogger, personRepo: PersonRepository, organisationRepo: OrganisationRepository, rolleRepo: RolleRepo, @@ -62,10 +65,12 @@ export class Personenkontext { personId, organisationId, rolleId, + logger ); } public static createNew( + logger: ClassLogger, personRepo: PersonRepository, organisationRepo: OrganisationRepository, rolleRepo: RolleRepo, @@ -83,6 +88,7 @@ export class Personenkontext { personId, organisationId, rolleId, + logger ); } @@ -123,8 +129,9 @@ export class Personenkontext { public async checkPermissions(permissions: PersonPermissions): Promise> { // Check if logged in person has permission + this.logger.info(`${permissions.personFields.id} START hasSystemrechteAtOrganisation`); { - const hasPermissionAtOrga: boolean = await permissions.hasSystemrechtAtOrganisation(this.organisationId, [ + const hasPermissionAtOrga: boolean = await permissions.hasSystemrechteAtOrganisation(this.organisationId, [ RollenSystemRecht.PERSONEN_VERWALTEN, ]); @@ -133,6 +140,7 @@ export class Personenkontext { return new MissingPermissionsError('Unauthorized to manage persons at the organisation'); } } + this.logger.info(`${permissions.personFields.id} END hasSystemrechteAtOrganisation`); // Check if logged in user can modify target person { diff --git a/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.ts b/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.ts index 196a0400f..edf918560 100644 --- a/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.ts +++ b/src/modules/personenkontext/persistence/dbiam-personenkontext.repo.ts @@ -122,7 +122,7 @@ export class DBiamPersonenkontextRepo { if (personenkontexte.length === 0) { const isAuthorizedAtRoot: boolean = - await permissions.hasSystemrechtAtRootOrganisation(relevantSystemRechte); + await permissions.hasSystemrechteAtRootOrganisation(relevantSystemRechte); if (!isAuthorizedAtRoot) { return { @@ -355,4 +355,38 @@ export class DBiamPersonenkontextRepo { rolleId: rolleId, }); } + + public async hasSystemrechtAtOrganisation( + personId: PersonID, + organisationId: OrganisationID, + systemrecht: RollenSystemRecht, + ): Promise { + const query: string = ` + WITH RECURSIVE parent_organisations AS ( + SELECT id, administriert_von + FROM public.organisation + WHERE id = ? + UNION ALL + SELECT o.id, o.administriert_von + FROM public.organisation o + INNER JOIN parent_organisations po ON o.id = po.administriert_von + ), + person_roles_at_orgas AS ( + SELECT DISTINCT pk.rolle_id + FROM parent_organisations po + JOIN public.personenkontext pk ON pk.organisation_id = po.id AND pk.person_id = ? + ) + SELECT EXISTS ( + SELECT 1 + FROM person_roles_at_orgas pr + JOIN public.rolle_systemrecht sr ON sr.rolle_id = pr.rolle_id + WHERE sr.systemrecht = ? + ) AS has_systemrecht_at_orga; + `; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result: any[] = await this.em.execute(query, [organisationId, personId, systemrecht]); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + return result[0].has_systemrecht_at_orga as boolean; + } } From 34c2562a8e36d3b925911734be276a35ba15c21a Mon Sep 17 00:00:00 2001 From: Caspar Neumann Date: Thu, 11 Jul 2024 11:36:42 +0200 Subject: [PATCH 2/5] Remove Logger --- .../personenkontext/domain/personenkontext.factory.ts | 4 ---- src/modules/personenkontext/domain/personenkontext.ts | 8 -------- 2 files changed, 12 deletions(-) diff --git a/src/modules/personenkontext/domain/personenkontext.factory.ts b/src/modules/personenkontext/domain/personenkontext.factory.ts index 31cf10b79..de1cb05a7 100644 --- a/src/modules/personenkontext/domain/personenkontext.factory.ts +++ b/src/modules/personenkontext/domain/personenkontext.factory.ts @@ -4,12 +4,10 @@ import { Personenkontext } from './personenkontext.js'; import { RolleRepo } from '../../rolle/repo/rolle.repo.js'; import { PersonRepository } from '../../person/persistence/person.repository.js'; import { OrganisationRepository } from '../../organisation/persistence/organisation.repository.js'; -import { ClassLogger } from '../../../core/logging/class-logger.js'; @Injectable() export class PersonenkontextFactory { public constructor( - private readonly logger: ClassLogger, private readonly personRepo: PersonRepository, private readonly organisationRepo: OrganisationRepository, private readonly rolleRepo: RolleRepo, @@ -24,7 +22,6 @@ export class PersonenkontextFactory { rolleId: RolleID, ): Personenkontext { return Personenkontext.construct( - this.logger, this.personRepo, this.organisationRepo, this.rolleRepo, @@ -39,7 +36,6 @@ export class PersonenkontextFactory { public createNew(personId: PersonID, organisationId: OrganisationID, rolleId: RolleID): Personenkontext { return Personenkontext.createNew( - this.logger, this.personRepo, this.organisationRepo, this.rolleRepo, diff --git a/src/modules/personenkontext/domain/personenkontext.ts b/src/modules/personenkontext/domain/personenkontext.ts index fbfbd121c..496420864 100644 --- a/src/modules/personenkontext/domain/personenkontext.ts +++ b/src/modules/personenkontext/domain/personenkontext.ts @@ -11,7 +11,6 @@ import { Rolle } from '../../rolle/domain/rolle.js'; import { RolleRepo } from '../../rolle/repo/rolle.repo.js'; import { OrganisationMatchesRollenart } from '../specification/organisation-matches-rollenart.js'; import { OrganisationMatchesRollenartError } from '../specification/error/organisation-matches-rollenart.error.js'; -import { ClassLogger } from '../../../core/logging/class-logger.js'; export type PersonenkontextPartial = Pick< Personenkontext, @@ -40,11 +39,9 @@ export class Personenkontext { public readonly personId: PersonID, public readonly organisationId: OrganisationID, public readonly rolleId: RolleID, - public readonly logger: ClassLogger, ) {} public static construct( - logger: ClassLogger, personRepo: PersonRepository, organisationRepo: OrganisationRepository, rolleRepo: RolleRepo, @@ -65,12 +62,10 @@ export class Personenkontext { personId, organisationId, rolleId, - logger ); } public static createNew( - logger: ClassLogger, personRepo: PersonRepository, organisationRepo: OrganisationRepository, rolleRepo: RolleRepo, @@ -88,7 +83,6 @@ export class Personenkontext { personId, organisationId, rolleId, - logger ); } @@ -129,7 +123,6 @@ export class Personenkontext { public async checkPermissions(permissions: PersonPermissions): Promise> { // Check if logged in person has permission - this.logger.info(`${permissions.personFields.id} START hasSystemrechteAtOrganisation`); { const hasPermissionAtOrga: boolean = await permissions.hasSystemrechteAtOrganisation(this.organisationId, [ RollenSystemRecht.PERSONEN_VERWALTEN, @@ -140,7 +133,6 @@ export class Personenkontext { return new MissingPermissionsError('Unauthorized to manage persons at the organisation'); } } - this.logger.info(`${permissions.personFields.id} END hasSystemrechteAtOrganisation`); // Check if logged in user can modify target person { From 8887f21e651747b151b1bbf15bdf81f3bf173080 Mon Sep 17 00:00:00 2001 From: Caspar Neumann Date: Thu, 11 Jul 2024 12:07:58 +0200 Subject: [PATCH 3/5] Complete Merge --- src/modules/person/domain/person.service.ts | 92 --------------------- 1 file changed, 92 deletions(-) diff --git a/src/modules/person/domain/person.service.ts b/src/modules/person/domain/person.service.ts index e59eeafcd..eb290e139 100644 --- a/src/modules/person/domain/person.service.ts +++ b/src/modules/person/domain/person.service.ts @@ -47,96 +47,4 @@ export class PersonService { items: persons, }; } -<<<<<<< HEAD - - public async createPersonWithPersonenkontext( - permissions: PersonPermissions, - vorname: string, - familienname: string, - organisationId: string, - rolleId: string, - ): Promise { - const person: Person | DomainError = await this.personFactory.createNew({ - vorname: vorname, - familienname: familienname, - }); - if (person instanceof DomainError) { - return person; - } - //Check references & ob der Admin berechtigt ist - const referenceError: Option = await this.checkReferences(organisationId, rolleId); - if (referenceError) { - return referenceError; - } - //Check Permissions für Personenkontext - const permissionsError: Option = await this.checkPermissions(permissions, organisationId); - if (permissionsError) { - return permissionsError; - } - //Save Person - const savedPerson: DomainError | Person = await this.personRepository.create(person); - if (savedPerson instanceof DomainError) { - return savedPerson; - } - - const personenkontext: Personenkontext = this.personenkontextFactory.createNew( - savedPerson.id, - organisationId, - rolleId, - ); - //Save Personenkontext - const savedPersonenkontext: Personenkontext = await this.personenkontextRepo.save(personenkontext); - return { - person: savedPerson, - personenkontext: savedPersonenkontext, - }; - } - - private async checkReferences(organisationId: string, rolleId: string): Promise> { - const [orga, rolle]: [Option>, Option>] = await Promise.all([ - this.organisationRepo.findById(organisationId), - this.rolleRepo.findById(rolleId), - ]); - - if (!orga) { - return new EntityNotFoundError('Organisation', organisationId); - } - - if (!rolle) { - return new EntityNotFoundError('Rolle', rolleId); - } - - // Can rolle be assigned at target orga - const canAssignRolle: boolean = await rolle.canBeAssignedToOrga(organisationId); - if (!canAssignRolle) { - return new EntityNotFoundError('rolle', rolleId); // Rolle does not exist for the chosen organisation - } - - //The aimed organisation needs to match the type of role to be assigned - const organisationMatchesRollenart: OrganisationMatchesRollenart = new OrganisationMatchesRollenart(); - if (!organisationMatchesRollenart.isSatisfiedBy(orga, rolle)) { - return new RolleNurAnPassendeOrganisationError(); - } - - return undefined; - } - - private async checkPermissions( - permissions: PersonPermissions, - organisationId: string, - ): Promise> { - // Check if logged in person has permission - const hasPermissionAtOrga: boolean = await permissions.hasSystemrechteAtOrganisation(organisationId, [ - RollenSystemRecht.PERSONEN_VERWALTEN, - ]); - - // Missing permission on orga - if (!hasPermissionAtOrga) { - return new MissingPermissionsError('Unauthorized to manage persons at the organisation'); - } - - return undefined; - } -======= ->>>>>>> main } From d4ec69da337a3b272801ceea93909b9efbf0e225 Mon Sep 17 00:00:00 2001 From: Caspar Neumann Date: Thu, 11 Jul 2024 12:18:07 +0200 Subject: [PATCH 4/5] Fix personpermissions.hasSystemrechtAtOrganisation --- ...ersonenkontext-workflow.controller.integration-spec.ts | 8 ++++---- .../personenkontext/domain/personenkontext-workflow.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/modules/personenkontext/api/dbiam-personenkontext-workflow.controller.integration-spec.ts b/src/modules/personenkontext/api/dbiam-personenkontext-workflow.controller.integration-spec.ts index 1de614391..d1f884cbd 100644 --- a/src/modules/personenkontext/api/dbiam-personenkontext-workflow.controller.integration-spec.ts +++ b/src/modules/personenkontext/api/dbiam-personenkontext-workflow.controller.integration-spec.ts @@ -173,7 +173,7 @@ describe('DbiamPersonenkontextWorkflowController Integration Test', () => { ); const personpermissions: DeepMocked = createMock(); - personpermissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); + personpermissions.hasSystemrechtAtOrganisation.mockResolvedValue(true); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); const response: Response = await request(app.getHttpServer() as App) @@ -195,7 +195,7 @@ describe('DbiamPersonenkontextWorkflowController Integration Test', () => { }), ); const permissions: DeepMocked = createMock(); - permissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); + permissions.hasSystemrechtAtOrganisation.mockResolvedValue(true); permissions.canModifyPerson.mockResolvedValueOnce(true); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(permissions); @@ -223,7 +223,7 @@ describe('DbiamPersonenkontextWorkflowController Integration Test', () => { ); const personpermissions: DeepMocked = createMock(); - personpermissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); + personpermissions.hasSystemrechtAtOrganisation.mockResolvedValue(true); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); const response: Response = await request(app.getHttpServer() as App) @@ -254,7 +254,7 @@ describe('DbiamPersonenkontextWorkflowController Integration Test', () => { ); const personpermissions: DeepMocked = createMock(); - personpermissions.hasSystemrechtAtOrganisation.mockResolvedValueOnce(false); + personpermissions.hasSystemrechtAtOrganisation.mockResolvedValue(false); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); const response: Response = await request(app.getHttpServer() as App) diff --git a/src/modules/personenkontext/domain/personenkontext-workflow.ts b/src/modules/personenkontext/domain/personenkontext-workflow.ts index 1001584ab..59f919c55 100644 --- a/src/modules/personenkontext/domain/personenkontext-workflow.ts +++ b/src/modules/personenkontext/domain/personenkontext-workflow.ts @@ -231,7 +231,7 @@ export class PersonenkontextWorkflowAggregate { organisationId: string, ): Promise> { // Check if logged in person has permission - const hasPermissionAtOrga: boolean = await permissions.hasSystemrechtAtOrganisation(organisationId, [ + const hasPermissionAtOrga: boolean = await permissions.hasSystemrechteAtOrganisation(organisationId, [ RollenSystemRecht.PERSONEN_VERWALTEN, ]); From 31cc4a43f93d7f2e96b990ebafc6a260374fabf0 Mon Sep 17 00:00:00 2001 From: Caspar Neumann Date: Thu, 11 Jul 2024 12:36:53 +0200 Subject: [PATCH 5/5] Fix Mocking --- ...am-personenkontext-workflow.controller.integration-spec.ts | 2 +- .../domain/personenkontext-creation.service.spec.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/personenkontext/api/dbiam-personenkontext-workflow.controller.integration-spec.ts b/src/modules/personenkontext/api/dbiam-personenkontext-workflow.controller.integration-spec.ts index d1f884cbd..1afb81907 100644 --- a/src/modules/personenkontext/api/dbiam-personenkontext-workflow.controller.integration-spec.ts +++ b/src/modules/personenkontext/api/dbiam-personenkontext-workflow.controller.integration-spec.ts @@ -254,7 +254,7 @@ describe('DbiamPersonenkontextWorkflowController Integration Test', () => { ); const personpermissions: DeepMocked = createMock(); - personpermissions.hasSystemrechtAtOrganisation.mockResolvedValue(false); + personpermissions.hasSystemrechteAtOrganisation.mockResolvedValueOnce(false); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); const response: Response = await request(app.getHttpServer() as App) diff --git a/src/modules/personenkontext/domain/personenkontext-creation.service.spec.ts b/src/modules/personenkontext/domain/personenkontext-creation.service.spec.ts index b4115b2ba..c409d2abb 100644 --- a/src/modules/personenkontext/domain/personenkontext-creation.service.spec.ts +++ b/src/modules/personenkontext/domain/personenkontext-creation.service.spec.ts @@ -195,7 +195,7 @@ describe('PersonenkontextCreationService', () => { organisationRepositoryMock.findById.mockResolvedValueOnce( createMock>({ typ: OrganisationsTyp.LAND }), ); - personpermissionsMock.hasSystemrechtAtOrganisation.mockResolvedValueOnce(false); + personpermissionsMock.hasSystemrechteAtOrganisation.mockResolvedValueOnce(false); const result: PersonPersonenkontext | DomainError = await sut.createPersonWithPersonenkontext( personpermissionsMock, @@ -215,7 +215,7 @@ describe('PersonenkontextCreationService', () => { organisationRepositoryMock.findById.mockResolvedValueOnce( createMock>({ typ: OrganisationsTyp.LAND }), ); - personpermissionsMock.hasSystemrechtAtOrganisation.mockResolvedValueOnce(true); + personpermissionsMock.hasSystemrechteAtOrganisation.mockResolvedValueOnce(true); personRepositoryMock.create.mockResolvedValueOnce( new KeycloakClientError('Username or email already exists'), );