From 01433fc585098ba70413d069205414893e1b4d4d Mon Sep 17 00:00:00 2001 From: Arne Gnisa Date: Tue, 19 Sep 2023 16:20:41 +0200 Subject: [PATCH] N21-1269 fix roster logic --- .../service/feathers-roster.service.spec.ts | 30 +++++++++++-------- .../service/feathers-roster.service.ts | 5 ++-- src/services/roster/index.js | 2 +- test/services/roster/index.test.js | 2 +- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/apps/server/src/modules/pseudonym/service/feathers-roster.service.spec.ts b/apps/server/src/modules/pseudonym/service/feathers-roster.service.spec.ts index e6467f1186c..12cbe9397a0 100644 --- a/apps/server/src/modules/pseudonym/service/feathers-roster.service.spec.ts +++ b/apps/server/src/modules/pseudonym/service/feathers-roster.service.spec.ts @@ -200,8 +200,10 @@ describe('FeathersRosterService', () => { describe('when pseudonym is given', () => { const setup = () => { const school: SchoolDO = schoolDOFactory.buildWithId(); - const externalTool: ExternalTool = externalToolFactory.buildWithId(); + const clientId = 'testClientId'; + const externalTool: ExternalTool = externalToolFactory.withOauth2Config({ clientId }).buildWithId(); const externalToolId: string = externalTool.id as string; + const otherExternalTool: ExternalTool = externalToolFactory.buildWithId(); const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId({ toolId: externalTool.id, @@ -239,6 +241,7 @@ describe('FeathersRosterService', () => { }); pseudonymService.findPseudonymByPseudonym.mockResolvedValue(pseudonym); + externalToolService.findExternalToolByOAuth2ConfigClientId.mockResolvedValue(externalTool); courseService.findAllByUserId.mockResolvedValue(courses); contextExternalToolService.findAllByContext.mockResolvedValueOnce([ contextExternalTool, @@ -254,6 +257,7 @@ describe('FeathersRosterService', () => { return { pseudonym, externalToolId, + clientId, user, courses, schoolExternalTool, @@ -263,25 +267,25 @@ describe('FeathersRosterService', () => { }; it('should call the pseudonym service to find the pseudonym', async () => { - const { pseudonym, externalToolId } = setup(); + const { pseudonym, clientId } = setup(); - await service.getUserGroups(pseudonym.pseudonym, externalToolId); + await service.getUserGroups(pseudonym.pseudonym, clientId); expect(pseudonymService.findPseudonymByPseudonym).toHaveBeenCalledWith(pseudonym.pseudonym); }); it('should call the course service to find the courses for the userId of the pseudonym', async () => { - const { pseudonym, externalToolId } = setup(); + const { pseudonym, clientId } = setup(); - await service.getUserGroups(pseudonym.pseudonym, externalToolId); + await service.getUserGroups(pseudonym.pseudonym, clientId); expect(courseService.findAllByUserId).toHaveBeenCalledWith(pseudonym.userId); }); it('should call the context external tool service to find the external tools for each course', async () => { - const { pseudonym, courses, externalToolId } = setup(); + const { pseudonym, courses, clientId } = setup(); - await service.getUserGroups(pseudonym.pseudonym, externalToolId); + await service.getUserGroups(pseudonym.pseudonym, clientId); expect(contextExternalToolService.findAllByContext.mock.calls).toEqual([ [new ContextRef({ id: courses[0].id, type: ToolContextType.COURSE })], @@ -291,9 +295,9 @@ describe('FeathersRosterService', () => { }); it('should call school external tool service to find the school external tool for each context external tool', async () => { - const { pseudonym, externalToolId, schoolExternalTool, otherSchoolExternalTool } = setup(); + const { pseudonym, clientId, schoolExternalTool, otherSchoolExternalTool } = setup(); - await service.getUserGroups(pseudonym.pseudonym, externalToolId); + await service.getUserGroups(pseudonym.pseudonym, clientId); expect(schoolExternalToolService.getSchoolExternalToolById.mock.calls).toEqual([ [schoolExternalTool.id], @@ -302,17 +306,17 @@ describe('FeathersRosterService', () => { }); it('should call external tool service to find the external tool for each school external tool', async () => { - const { pseudonym, externalToolId, otherExternalTool } = setup(); + const { pseudonym, clientId, otherExternalTool, externalToolId } = setup(); - await service.getUserGroups(pseudonym.pseudonym, externalToolId); + await service.getUserGroups(pseudonym.pseudonym, clientId); expect(externalToolService.findExternalToolById.mock.calls).toEqual([[externalToolId], [otherExternalTool.id]]); }); it('should return a group for each course where the tool of the users pseudonym is used', async () => { - const { pseudonym, externalToolId, courses } = setup(); + const { pseudonym, clientId, courses } = setup(); - const result = await service.getUserGroups(pseudonym.pseudonym, externalToolId); + const result = await service.getUserGroups(pseudonym.pseudonym, clientId); expect(result).toEqual({ data: { diff --git a/apps/server/src/modules/pseudonym/service/feathers-roster.service.ts b/apps/server/src/modules/pseudonym/service/feathers-roster.service.ts index 0951c0dd18c..2b7b8b8b2bb 100644 --- a/apps/server/src/modules/pseudonym/service/feathers-roster.service.ts +++ b/apps/server/src/modules/pseudonym/service/feathers-roster.service.ts @@ -75,11 +75,12 @@ export class FeathersRosterService { return userMetadata; } - async getUserGroups(pseudonym: string, externalToolId: string): Promise { + async getUserGroups(pseudonym: string, oauth2ClientId: string): Promise { const loadedPseudonym: Pseudonym = await this.findPseudonymByPseudonym(pseudonym); + const externalTool: ExternalTool = await this.validateAndGetExternalTool(oauth2ClientId); let courses: Course[] = await this.getCoursesFromUsersPseudonym(loadedPseudonym); - courses = await this.filterCoursesByToolAvailability(courses, externalToolId); + courses = await this.filterCoursesByToolAvailability(courses, externalTool.id as string); const userGroups: UserGroups = { data: { diff --git a/src/services/roster/index.js b/src/services/roster/index.js index f9f9de615ad..182f2e74a4e 100644 --- a/src/services/roster/index.js +++ b/src/services/roster/index.js @@ -92,7 +92,7 @@ module.exports = function roster() { if (Configuration.get('FEATURE_CTL_TOOLS_TAB_ENABLED')) { const userGroups = await app .service('nest-feathers-roster-service') - .getUserGroups(params.pseudonym, params.originToolId); + .getUserGroups(params.pseudonym, params.tokenInfo.client_id); return userGroups; } diff --git a/test/services/roster/index.test.js b/test/services/roster/index.test.js index 757f0ca5f40..791ad6b627f 100644 --- a/test/services/roster/index.test.js +++ b/test/services/roster/index.test.js @@ -11,7 +11,7 @@ const { chai.use(chaiHttp); -describe.only('roster service', function oauth() { +describe('roster service', function oauth() { let app; let metadataService; let userGroupsService;