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/personenkontext/api/dbiam-personenkontext-workflow.controller.integration-spec.ts b/src/modules/personenkontext/api/dbiam-personenkontext-workflow.controller.integration-spec.ts index 1de614391..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 @@ -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.hasSystemrechteAtOrganisation.mockResolvedValueOnce(false); personpermissionsRepoMock.loadPersonPermissions.mockResolvedValue(personpermissions); const response: Response = await request(app.getHttpServer() as App) 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 f5044985a..803c0ca04 100644 --- a/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration.spec.ts +++ b/src/modules/personenkontext/api/dbiam-personenkontext.controller.integration.spec.ts @@ -189,7 +189,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()}`) @@ -221,7 +221,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') @@ -253,7 +253,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') @@ -282,7 +282,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) @@ -408,7 +408,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') @@ -439,7 +439,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-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'), ); 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, ]); 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..496420864 100644 --- a/src/modules/personenkontext/domain/personenkontext.ts +++ b/src/modules/personenkontext/domain/personenkontext.ts @@ -124,7 +124,7 @@ export class Personenkontext { public async checkPermissions(permissions: PersonPermissions): Promise> { // Check if logged in person has permission { - const hasPermissionAtOrga: boolean = await permissions.hasSystemrechtAtOrganisation(this.organisationId, [ + const hasPermissionAtOrga: boolean = await permissions.hasSystemrechteAtOrganisation(this.organisationId, [ RollenSystemRecht.PERSONEN_VERWALTEN, ]); 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; + } }