From 63f38be55227c83660bb31cff110fc7cdf9f5ab3 Mon Sep 17 00:00:00 2001 From: mamutmk5 <3045922+mamutmk5@users.noreply.github.com> Date: Thu, 2 Nov 2023 10:28:36 +0100 Subject: [PATCH 1/8] BC-5546 - Split ingress for Domains (#4495) Separate the Server Ingress to the Server Repo and for each service in an own one. Add the new ingress definitons files to the ansible roles. With the current version of nginx ingress is it possible to have more igresses with different resources for one domain. --- .../schulcloud-server-core/tasks/main.yml | 28 +++++++++++++ .../templates/api-files-ingress.yml.j2 | 41 +++++++++++++++++++ .../templates/api-fwu-ingress.yml.j2 | 41 +++++++++++++++++++ .../templates/ingress.yml.j2 | 41 +++++++++++++++++++ .../schulcloud-server-h5p/tasks/main.yml | 8 ++++ .../templates/api-h5p-ingress.yml.j2 | 41 +++++++++++++++++++ 6 files changed, 200 insertions(+) create mode 100644 ansible/roles/schulcloud-server-core/templates/api-files-ingress.yml.j2 create mode 100644 ansible/roles/schulcloud-server-core/templates/api-fwu-ingress.yml.j2 create mode 100644 ansible/roles/schulcloud-server-core/templates/ingress.yml.j2 create mode 100644 ansible/roles/schulcloud-server-h5p/templates/api-h5p-ingress.yml.j2 diff --git a/ansible/roles/schulcloud-server-core/tasks/main.yml b/ansible/roles/schulcloud-server-core/tasks/main.yml index 7f1bbeeecfe..1b58c8a5413 100644 --- a/ansible/roles/schulcloud-server-core/tasks/main.yml +++ b/ansible/roles/schulcloud-server-core/tasks/main.yml @@ -58,6 +58,13 @@ kubeconfig: ~/.kube/config namespace: "{{ NAMESPACE }}" template: deployment.yml.j2 + + - name: Ingress + kubernetes.core.k8s: + kubeconfig: ~/.kube/config + namespace: "{{ NAMESPACE }}" + template: ingress.yml.j2 + apply: yes - name: FileStorageDeployment kubernetes.core.k8s: @@ -65,6 +72,19 @@ namespace: "{{ NAMESPACE }}" template: api-files-deployment.yml.j2 + - name: FileStorageDeployment + kubernetes.core.k8s: + kubeconfig: ~/.kube/config + namespace: "{{ NAMESPACE }}" + template: api-files-deployment.yml.j2 + + - name: File Storage Ingress + kubernetes.core.k8s: + kubeconfig: ~/.kube/config + namespace: "{{ NAMESPACE }}" + template: api-files-ingress.yml.j2 + apply: yes + - name: FwuLearningContentsDeployment kubernetes.core.k8s: kubeconfig: ~/.kube/config @@ -72,6 +92,14 @@ template: api-fwu-deployment.yml.j2 when: FEATURE_FWU_CONTENT_ENABLED is defined and FEATURE_FWU_CONTENT_ENABLED|bool + - name: Fwu Learning Contents Ingress + kubernetes.core.k8s: + kubeconfig: ~/.kube/config + namespace: "{{ NAMESPACE }}" + template: api-fwu-ingress.yml.j2 + apply: yes + when: FEATURE_FWU_CONTENT_ENABLED is defined and FEATURE_FWU_CONTENT_ENABLED|bool + - name: Delete Files CronJob kubernetes.core.k8s: kubeconfig: ~/.kube/config diff --git a/ansible/roles/schulcloud-server-core/templates/api-files-ingress.yml.j2 b/ansible/roles/schulcloud-server-core/templates/api-files-ingress.yml.j2 new file mode 100644 index 00000000000..a1264b52001 --- /dev/null +++ b/ansible/roles/schulcloud-server-core/templates/api-files-ingress.yml.j2 @@ -0,0 +1,41 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: {{ NAMESPACE }}-api-files-ingress + namespace: {{ NAMESPACE }} + annotations: + nginx.ingress.kubernetes.io/ssl-redirect: "{{ TLS_ENABELD|default("false") }}" + nginx.ingress.kubernetes.io/proxy-body-size: "{{ INGRESS_MAX_BODY_SIZE|default("2560") }}m" + nginx.org/client-max-body-size: "{{ INGRESS_MAX_BODY_SIZE|default("2560") }}m" + # The following properties added with BC-3606. + # The header size of the request is too big. For e.g. state and the permanent growing jwt. + # Nginx throws away the Location header, resulting in the 502 Bad Gateway. + nginx.ingress.kubernetes.io/client-header-buffer-size: 100k + nginx.ingress.kubernetes.io/http2-max-header-size: 96k + nginx.ingress.kubernetes.io/large-client-header-buffers: 4 100k + nginx.ingress.kubernetes.io/proxy-buffer-size: 96k +{% if CLUSTER_ISSUER is defined %} + cert-manager.io/cluster-issuer: {{ CLUSTER_ISSUER }} +{% endif %} + +spec: + ingressClassName: nginx +{% if CLUSTER_ISSUER is defined or (TLS_ENABELD is defined and TLS_ENABELD|bool) %} + tls: + - hosts: + - {{ DOMAIN }} +{% if CLUSTER_ISSUER is defined %} + secretName: {{ DOMAIN }}-tls +{% endif %} +{% endif %} + rules: + - host: {{ DOMAIN }} + http: + paths: + - path: /api/v3/file/ + backend: + service: + name: api-files-svc + port: + number: {{ PORT_FILE_SERVICE }} + pathType: Prefix diff --git a/ansible/roles/schulcloud-server-core/templates/api-fwu-ingress.yml.j2 b/ansible/roles/schulcloud-server-core/templates/api-fwu-ingress.yml.j2 new file mode 100644 index 00000000000..f42c322e45b --- /dev/null +++ b/ansible/roles/schulcloud-server-core/templates/api-fwu-ingress.yml.j2 @@ -0,0 +1,41 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: {{ NAMESPACE }}-api-fwu-ingress + namespace: {{ NAMESPACE }} + annotations: + nginx.ingress.kubernetes.io/ssl-redirect: "{{ TLS_ENABELD|default("false") }}" + nginx.ingress.kubernetes.io/proxy-body-size: "{{ INGRESS_MAX_BODY_SIZE|default("2560") }}m" + nginx.org/client-max-body-size: "{{ INGRESS_MAX_BODY_SIZE|default("2560") }}m" + # The following properties added with BC-3606. + # The header size of the request is too big. For e.g. state and the permanent growing jwt. + # Nginx throws away the Location header, resulting in the 502 Bad Gateway. + nginx.ingress.kubernetes.io/client-header-buffer-size: 100k + nginx.ingress.kubernetes.io/http2-max-header-size: 96k + nginx.ingress.kubernetes.io/large-client-header-buffers: 4 100k + nginx.ingress.kubernetes.io/proxy-buffer-size: 96k +{% if CLUSTER_ISSUER is defined %} + cert-manager.io/cluster-issuer: {{ CLUSTER_ISSUER }} +{% endif %} + +spec: + ingressClassName: nginx +{% if CLUSTER_ISSUER is defined or (TLS_ENABELD is defined and TLS_ENABELD|bool) %} + tls: + - hosts: + - {{ DOMAIN }} +{% if CLUSTER_ISSUER is defined %} + secretName: {{ DOMAIN }}-tls +{% endif %} +{% endif %} + rules: + - host: {{ DOMAIN }} + http: + paths: + - path: /api/v3/fwu/ + backend: + service: + name: api-fwu-svc + port: + number: {{ PORT_FWU_LEARNING_CONTENTS }} + pathType: Prefix diff --git a/ansible/roles/schulcloud-server-core/templates/ingress.yml.j2 b/ansible/roles/schulcloud-server-core/templates/ingress.yml.j2 new file mode 100644 index 00000000000..b2dd208765f --- /dev/null +++ b/ansible/roles/schulcloud-server-core/templates/ingress.yml.j2 @@ -0,0 +1,41 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: {{ NAMESPACE }}-api-ingress + namespace: {{ NAMESPACE }} + annotations: + nginx.ingress.kubernetes.io/ssl-redirect: "{{ TLS_ENABELD|default("false") }}" + nginx.ingress.kubernetes.io/proxy-body-size: "{{ INGRESS_MAX_BODY_SIZE|default("2560") }}m" + nginx.org/client-max-body-size: "{{ INGRESS_MAX_BODY_SIZE|default("2560") }}m" + # The following properties added with BC-3606. + # The header size of the request is too big. For e.g. state and the permanent growing jwt. + # Nginx throws away the Location header, resulting in the 502 Bad Gateway. + nginx.ingress.kubernetes.io/client-header-buffer-size: 100k + nginx.ingress.kubernetes.io/http2-max-header-size: 96k + nginx.ingress.kubernetes.io/large-client-header-buffers: 4 100k + nginx.ingress.kubernetes.io/proxy-buffer-size: 96k +{% if CLUSTER_ISSUER is defined %} + cert-manager.io/cluster-issuer: {{ CLUSTER_ISSUER }} +{% endif %} + +spec: + ingressClassName: nginx +{% if CLUSTER_ISSUER is defined or (TLS_ENABELD is defined and TLS_ENABELD|bool) %} + tls: + - hosts: + - {{ DOMAIN }} +{% if CLUSTER_ISSUER is defined %} + secretName: {{ DOMAIN }}-tls +{% endif %} +{% endif %} + rules: + - host: {{ DOMAIN }} + http: + paths: + - path: /api/v3/ + backend: + service: + name: api-svc + port: + number: {{ PORT_SERVER }} + pathType: Prefix diff --git a/ansible/roles/schulcloud-server-h5p/tasks/main.yml b/ansible/roles/schulcloud-server-h5p/tasks/main.yml index f630b1f3671..368e97a216e 100644 --- a/ansible/roles/schulcloud-server-h5p/tasks/main.yml +++ b/ansible/roles/schulcloud-server-h5p/tasks/main.yml @@ -11,4 +11,12 @@ namespace: "{{ NAMESPACE }}" template: api-h5p-deployment.yml.j2 when: WITH_H5P_EDITOR is defined and WITH_H5P_EDITOR|bool + + - name: H5p Editor Ingress + kubernetes.core.k8s: + kubeconfig: ~/.kube/config + namespace: "{{ NAMESPACE }}" + template: api-h5p-ingress.yml.j2 + apply: yes + when: WITH_H5P_EDITOR is defined and WITH_H5P_EDITOR|bool \ No newline at end of file diff --git a/ansible/roles/schulcloud-server-h5p/templates/api-h5p-ingress.yml.j2 b/ansible/roles/schulcloud-server-h5p/templates/api-h5p-ingress.yml.j2 new file mode 100644 index 00000000000..ec68641bfa2 --- /dev/null +++ b/ansible/roles/schulcloud-server-h5p/templates/api-h5p-ingress.yml.j2 @@ -0,0 +1,41 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: {{ NAMESPACE }}-api-h5p-ingress + namespace: {{ NAMESPACE }} + annotations: + nginx.ingress.kubernetes.io/ssl-redirect: "{{ TLS_ENABELD|default("false") }}" + nginx.ingress.kubernetes.io/proxy-body-size: "{{ INGRESS_MAX_BODY_SIZE|default("2560") }}m" + nginx.org/client-max-body-size: "{{ INGRESS_MAX_BODY_SIZE|default("2560") }}m" + # The following properties added with BC-3606. + # The header size of the request is too big. For e.g. state and the permanent growing jwt. + # Nginx throws away the Location header, resulting in the 502 Bad Gateway. + nginx.ingress.kubernetes.io/client-header-buffer-size: 100k + nginx.ingress.kubernetes.io/http2-max-header-size: 96k + nginx.ingress.kubernetes.io/large-client-header-buffers: 4 100k + nginx.ingress.kubernetes.io/proxy-buffer-size: 96k +{% if CLUSTER_ISSUER is defined %} + cert-manager.io/cluster-issuer: {{ CLUSTER_ISSUER }} +{% endif %} + +spec: + ingressClassName: nginx +{% if CLUSTER_ISSUER is defined or (TLS_ENABELD is defined and TLS_ENABELD|bool) %} + tls: + - hosts: + - {{ DOMAIN }} +{% if CLUSTER_ISSUER is defined %} + secretName: {{ DOMAIN }}-tls +{% endif %} +{% endif %} + rules: + - host: {{ DOMAIN }} + http: + paths: + - path: /api/v3/h5p-editor/ + backend: + service: + name: api-h5p-svc + port: + number: 4448 + pathType: Prefix From c4b1288e8923a234635fb8c2f6bc13396742244d Mon Sep 17 00:00:00 2001 From: agnisa-cap Date: Thu, 2 Nov 2023 15:33:23 +0100 Subject: [PATCH 2/8] N21-1219 hide email section (#4455) * adds isExternalUser to current user --- .../authentication/interface/jwt-payload.ts | 1 + .../modules/authentication/interface/user.ts | 3 ++ .../mapper/current-user.mapper.spec.ts | 52 ++++++++++++++++++- .../mapper/current-user.mapper.ts | 4 ++ .../services/authentication.service.spec.ts | 1 + .../strategy/ldap.strategy.spec.ts | 2 + .../strategy/oauth2.strategy.spec.ts | 1 + .../authentication/uc/login.uc.spec.ts | 2 + .../src/modules/oauth/uc/oauth.uc.spec.ts | 22 ++++---- apps/server/src/modules/oauth/uc/oauth.uc.ts | 6 +-- .../modules/user/service/user.service.spec.ts | 35 ++++++++----- .../src/modules/user/service/user.service.ts | 20 +++---- .../uc/video-conference-deprecated.uc.spec.ts | 1 + .../testing/map-user-to-current-user.ts | 1 + 14 files changed, 111 insertions(+), 40 deletions(-) diff --git a/apps/server/src/modules/authentication/interface/jwt-payload.ts b/apps/server/src/modules/authentication/interface/jwt-payload.ts index aad11700e60..ca46acbe761 100644 --- a/apps/server/src/modules/authentication/interface/jwt-payload.ts +++ b/apps/server/src/modules/authentication/interface/jwt-payload.ts @@ -6,6 +6,7 @@ export interface CreateJwtPayload { systemId?: string; // without this the user needs to change his PW during first login support?: boolean; // support UserId is missed see featherJS + isExternalUser: boolean; } export interface JwtPayload extends CreateJwtPayload { diff --git a/apps/server/src/modules/authentication/interface/user.ts b/apps/server/src/modules/authentication/interface/user.ts index a070367a43b..cc8423f69b7 100644 --- a/apps/server/src/modules/authentication/interface/user.ts +++ b/apps/server/src/modules/authentication/interface/user.ts @@ -15,6 +15,9 @@ export interface ICurrentUser { /** True if a support member impersonates the user */ impersonated?: boolean; + + /** True if the user is an external user e.g. an oauth user */ + isExternalUser: boolean; } export interface OauthCurrentUser extends ICurrentUser { diff --git a/apps/server/src/modules/authentication/mapper/current-user.mapper.spec.ts b/apps/server/src/modules/authentication/mapper/current-user.mapper.spec.ts index 09a76c1ebfb..d06bea6d080 100644 --- a/apps/server/src/modules/authentication/mapper/current-user.mapper.spec.ts +++ b/apps/server/src/modules/authentication/mapper/current-user.mapper.spec.ts @@ -61,6 +61,7 @@ describe('CurrentUserMapper', () => { describe('when userDO has no ID', () => { it('should throw error', () => { const user: UserDO = userDoFactory.build({ createdAt: new Date(), updatedAt: new Date() }); + expect(() => CurrentUserMapper.mapToOauthCurrentUser(accountId, user, undefined, 'idToken')).toThrow( ValidationError ); @@ -100,6 +101,7 @@ describe('CurrentUserMapper', () => { schoolId: user.schoolId, userId, externalIdToken: idToken, + isExternalUser: true, }); }); }); @@ -139,6 +141,7 @@ describe('CurrentUserMapper', () => { schoolId: user.schoolId, userId, externalIdToken: idToken, + isExternalUser: true, }); }); }); @@ -181,7 +184,7 @@ describe('CurrentUserMapper', () => { describe('jwtToICurrentUser', () => { describe('when JWT is provided with all claims', () => { - it('should return current user', () => { + const setup = () => { const jwtPayload: JwtPayload = { accountId: 'dummyAccountId', systemId: 'dummySystemId', @@ -189,6 +192,7 @@ describe('CurrentUserMapper', () => { schoolId: 'dummySchoolId', userId: 'dummyUserId', support: true, + isExternalUser: true, sub: 'dummyAccountId', jti: 'random string', aud: 'some audience', @@ -196,7 +200,17 @@ describe('CurrentUserMapper', () => { iat: Math.floor(new Date().getTime() / 1000), exp: Math.floor(new Date().getTime() / 1000) + 3600, }; + + return { + jwtPayload, + }; + }; + + it('should return current user', () => { + const { jwtPayload } = setup(); + const currentUser = CurrentUserMapper.jwtToICurrentUser(jwtPayload); + expect(currentUser).toMatchObject({ accountId: jwtPayload.accountId, systemId: jwtPayload.systemId, @@ -206,15 +220,26 @@ describe('CurrentUserMapper', () => { impersonated: jwtPayload.support, }); }); + + it('should return current user with default for isExternalUser', () => { + const { jwtPayload } = setup(); + + const currentUser = CurrentUserMapper.jwtToICurrentUser(jwtPayload); + + expect(currentUser).toMatchObject({ + isExternalUser: jwtPayload.isExternalUser, + }); + }); }); describe('when JWT is provided without optional claims', () => { - it('should return current user', () => { + const setup = () => { const jwtPayload: JwtPayload = { accountId: 'dummyAccountId', roles: ['mockRoleId'], schoolId: 'dummySchoolId', userId: 'dummyUserId', + isExternalUser: false, sub: 'dummyAccountId', jti: 'random string', aud: 'some audience', @@ -222,12 +247,33 @@ describe('CurrentUserMapper', () => { iat: Math.floor(new Date().getTime() / 1000), exp: Math.floor(new Date().getTime() / 1000) + 3600, }; + + return { + jwtPayload, + }; + }; + + it('should return current user', () => { + const { jwtPayload } = setup(); + const currentUser = CurrentUserMapper.jwtToICurrentUser(jwtPayload); + expect(currentUser).toMatchObject({ accountId: jwtPayload.accountId, roles: [jwtPayload.roles[0]], schoolId: jwtPayload.schoolId, userId: jwtPayload.userId, + isExternalUser: false, + }); + }); + + it('should return current user with default for isExternalUser', () => { + const { jwtPayload } = setup(); + + const currentUser = CurrentUserMapper.jwtToICurrentUser(jwtPayload); + + expect(currentUser).toMatchObject({ + isExternalUser: false, }); }); }); @@ -242,6 +288,7 @@ describe('CurrentUserMapper', () => { schoolId: 'dummySchoolId', userId: 'dummyUserId', impersonated: true, + isExternalUser: false, }; const createJwtPayload: CreateJwtPayload = CurrentUserMapper.mapCurrentUserToCreateJwtPayload(currentUser); @@ -253,6 +300,7 @@ describe('CurrentUserMapper', () => { schoolId: currentUser.schoolId, userId: currentUser.userId, support: currentUser.impersonated, + isExternalUser: false, }); }); }); diff --git a/apps/server/src/modules/authentication/mapper/current-user.mapper.ts b/apps/server/src/modules/authentication/mapper/current-user.mapper.ts index 80ca91b56b0..ab832b70d8c 100644 --- a/apps/server/src/modules/authentication/mapper/current-user.mapper.ts +++ b/apps/server/src/modules/authentication/mapper/current-user.mapper.ts @@ -13,6 +13,7 @@ export class CurrentUserMapper { roles: user.roles.getItems().map((role: Role) => role.id), schoolId: user.school.id, userId: user.id, + isExternalUser: false, }; } @@ -33,6 +34,7 @@ export class CurrentUserMapper { schoolId: user.schoolId, userId: user.id, externalIdToken, + isExternalUser: true, }; } @@ -44,6 +46,7 @@ export class CurrentUserMapper { roles: currentUser.roles, systemId: currentUser.systemId, support: currentUser.impersonated, + isExternalUser: currentUser.isExternalUser, }; } @@ -55,6 +58,7 @@ export class CurrentUserMapper { schoolId: jwtPayload.schoolId, userId: jwtPayload.userId, impersonated: jwtPayload.support, + isExternalUser: jwtPayload.isExternalUser, }; } } diff --git a/apps/server/src/modules/authentication/services/authentication.service.spec.ts b/apps/server/src/modules/authentication/services/authentication.service.spec.ts index 3d5b6d3a1b7..1e5c69ecfb1 100644 --- a/apps/server/src/modules/authentication/services/authentication.service.spec.ts +++ b/apps/server/src/modules/authentication/services/authentication.service.spec.ts @@ -99,6 +99,7 @@ describe('AuthenticationService', () => { roles: ['student'], schoolId: 'mockSchoolId', userId: 'mockUserId', + isExternalUser: false, }; await authenticationService.generateJwt(mockCurrentUser); expect(jwtService.sign).toBeCalledWith( diff --git a/apps/server/src/modules/authentication/strategy/ldap.strategy.spec.ts b/apps/server/src/modules/authentication/strategy/ldap.strategy.spec.ts index d686dfcac72..78f445ce5b0 100644 --- a/apps/server/src/modules/authentication/strategy/ldap.strategy.spec.ts +++ b/apps/server/src/modules/authentication/strategy/ldap.strategy.spec.ts @@ -436,6 +436,7 @@ describe('LdapStrategy', () => { schoolId: school.id, systemId: system.id, accountId: account.id, + isExternalUser: false, }); }); }); @@ -500,6 +501,7 @@ describe('LdapStrategy', () => { schoolId: school.id, systemId: system.id, accountId: account.id, + isExternalUser: false, }); }); }); diff --git a/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts b/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts index 8fd8f096dbe..f67f620175d 100644 --- a/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts +++ b/apps/server/src/modules/authentication/strategy/oauth2.strategy.spec.ts @@ -88,6 +88,7 @@ describe('Oauth2Strategy', () => { schoolId: user.schoolId, accountId: account.id, externalIdToken: idToken, + isExternalUser: true, }); }); }); diff --git a/apps/server/src/modules/authentication/uc/login.uc.spec.ts b/apps/server/src/modules/authentication/uc/login.uc.spec.ts index a14b741ae88..c0f1d924876 100644 --- a/apps/server/src/modules/authentication/uc/login.uc.spec.ts +++ b/apps/server/src/modules/authentication/uc/login.uc.spec.ts @@ -35,6 +35,7 @@ describe('LoginUc', () => { userId: '', systemId: '', impersonated: false, + isExternalUser: false, someProperty: 'shouldNotBeMapped', }; const loginDto: LoginDto = new LoginDto({ accessToken: 'accessToken' }); @@ -58,6 +59,7 @@ describe('LoginUc', () => { roles: userInfo.roles, systemId: userInfo.systemId, support: userInfo.impersonated, + isExternalUser: userInfo.isExternalUser, }); }); diff --git a/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts b/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts index 4323cd5bc85..1e888abd5f1 100644 --- a/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts +++ b/apps/server/src/modules/oauth/uc/oauth.uc.spec.ts @@ -1,17 +1,9 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { UnauthorizedException, UnprocessableEntityException } from '@nestjs/common'; -import { Test, TestingModule } from '@nestjs/testing'; -import { LegacySchoolDo, UserDO } from '@shared/domain'; -import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; -import { ISession } from '@shared/domain/types/session'; -import { legacySchoolDoFactory, setupEntities } from '@shared/testing'; -import { LegacyLogger } from '@src/core/logger'; -import { ICurrentUser } from '@modules/authentication'; import { AuthenticationService } from '@modules/authentication/services/authentication.service'; +import { LegacySchoolService } from '@modules/legacy-school'; import { OauthUc } from '@modules/oauth/uc/oauth.uc'; import { ProvisioningService } from '@modules/provisioning'; import { ExternalUserDto, OauthDataDto, ProvisioningSystemDto } from '@modules/provisioning/dto'; -import { LegacySchoolService } from '@modules/legacy-school'; import { SystemService } from '@modules/system'; import { OauthConfigDto, SystemDto } from '@modules/system/service'; import { UserService } from '@modules/user'; @@ -19,9 +11,17 @@ import { UserMigrationService } from '@modules/user-login-migration'; import { OAuthMigrationError } from '@modules/user-login-migration/error/oauth-migration.error'; import { SchoolMigrationService } from '@modules/user-login-migration/service'; import { MigrationDto } from '@modules/user-login-migration/service/dto'; -import { OAuthSSOError } from '../loggable/oauth-sso.error'; +import { UnauthorizedException, UnprocessableEntityException } from '@nestjs/common'; +import { Test, TestingModule } from '@nestjs/testing'; +import { LegacySchoolDo, UserDO } from '@shared/domain'; +import { SystemProvisioningStrategy } from '@shared/domain/interface/system-provisioning.strategy'; +import { ISession } from '@shared/domain/types/session'; +import { legacySchoolDoFactory, setupEntities } from '@shared/testing'; +import { LegacyLogger } from '@src/core/logger'; +import { OauthCurrentUser } from '@modules/authentication/interface'; import { AuthorizationParams } from '../controller/dto'; import { OAuthTokenDto } from '../interface'; +import { OAuthSSOError } from '../loggable/oauth-sso.error'; import { OAuthProcessDto } from '../service/dto'; import { OAuthService } from '../service/oauth.service'; import { OauthLoginStateDto } from './dto/oauth-login-state.dto'; @@ -254,7 +254,7 @@ describe('OAuthUc', () => { externalId: 'mockExternalId', }); - const currentUser: ICurrentUser = { userId: 'userId' } as ICurrentUser; + const currentUser: OauthCurrentUser = { userId: 'userId', isExternalUser: true } as OauthCurrentUser; const testSystem: SystemDto = new SystemDto({ id: 'mockSystemId', type: 'mock', diff --git a/apps/server/src/modules/oauth/uc/oauth.uc.ts b/apps/server/src/modules/oauth/uc/oauth.uc.ts index 53d986bf029..c495e7be05d 100644 --- a/apps/server/src/modules/oauth/uc/oauth.uc.ts +++ b/apps/server/src/modules/oauth/uc/oauth.uc.ts @@ -2,7 +2,6 @@ import { Injectable, UnauthorizedException, UnprocessableEntityException } from import { EntityId, LegacySchoolDo, UserDO } from '@shared/domain'; import { ISession } from '@shared/domain/types/session'; import { LegacyLogger } from '@src/core/logger'; -import { ICurrentUser } from '@modules/authentication'; import { AuthenticationService } from '@modules/authentication/services/authentication.service'; import { ProvisioningService } from '@modules/provisioning'; import { OauthDataDto } from '@modules/provisioning/dto'; @@ -13,6 +12,7 @@ import { UserMigrationService } from '@modules/user-login-migration'; import { SchoolMigrationService } from '@modules/user-login-migration/service'; import { MigrationDto } from '@modules/user-login-migration/service/dto'; import { nanoid } from 'nanoid'; +import { OauthCurrentUser } from '@modules/authentication/interface'; import { AuthorizationParams } from '../controller/dto'; import { OAuthTokenDto } from '../interface'; import { OAuthProcessDto } from '../service/dto'; @@ -140,9 +140,9 @@ export class OauthUc { } private async getJwtForUser(userId: EntityId): Promise { - const currentUser: ICurrentUser = await this.userService.getResolvedUser(userId); + const oauthCurrentUser: OauthCurrentUser = await this.userService.getResolvedUser(userId); - const { accessToken } = await this.authenticationService.generateJwt(currentUser); + const { accessToken } = await this.authenticationService.generateJwt(oauthCurrentUser); return accessToken; } diff --git a/apps/server/src/modules/user/service/user.service.spec.ts b/apps/server/src/modules/user/service/user.service.spec.ts index 1b208764c8f..223fa1f0c88 100644 --- a/apps/server/src/modules/user/service/user.service.spec.ts +++ b/apps/server/src/modules/user/service/user.service.spec.ts @@ -9,10 +9,10 @@ import { UserDORepo } from '@shared/repo/user/user-do.repo'; import { roleFactory, setupEntities, userDoFactory, userFactory } from '@shared/testing'; import { AccountService } from '@modules/account/services/account.service'; import { AccountDto } from '@modules/account/services/dto'; -import { ICurrentUser } from '@modules/authentication'; import { RoleService } from '@modules/role/service/role.service'; import { UserService } from '@modules/user/service/user.service'; import { UserDto } from '@modules/user/uc/dto/user.dto'; +import { OauthCurrentUser } from '@modules/authentication/interface'; import { UserQuery } from './user-query.type'; describe('UserService', () => { @@ -136,13 +136,13 @@ describe('UserService', () => { describe('getResolvedUser is called', () => { describe('when a resolved user is requested', () => { - it('should return an ICurrentUser', async () => { + const setup = () => { const systemId = 'systemId'; const role: Role = roleFactory.buildWithId({ name: RoleName.STUDENT, permissions: [Permission.DASHBOARD_VIEW], }); - const user: User = userFactory.buildWithId({ roles: [role] }); + const user: UserDO = userDoFactory.buildWithId({ roles: [role] }); const account: AccountDto = new AccountDto({ id: 'accountId', systemId, @@ -152,17 +152,30 @@ describe('UserService', () => { activated: true, }); - userRepo.findById.mockResolvedValue(user); + userDORepo.findById.mockResolvedValue(user); accountService.findByUserIdOrFail.mockResolvedValue(account); - const result: ICurrentUser = await service.getResolvedUser(user.id); + return { + userId: user.id as string, + user, + account, + role, + systemId, + }; + }; + + it('should return the current user', async () => { + const { userId, user, account, role, systemId } = setup(); - expect(result).toEqual({ - userId: user.id, + const result: OauthCurrentUser = await service.getResolvedUser(userId); + + expect(result).toEqual({ + userId, systemId, - schoolId: user.school.id, + schoolId: user.schoolId, accountId: account.id, roles: [role.id], + isExternalUser: true, }); }); }); @@ -177,30 +190,24 @@ describe('UserService', () => { }); it('should return only the last name when the user has a protected role', async () => { - // Arrange const user: UserDO = userDoFactory.withRoles([{ id: role.id, name: RoleName.STUDENT }]).buildWithId({ lastName: 'lastName', }); - // Act const result: string = await service.getDisplayName(user); - // Assert expect(result).toEqual(user.lastName); expect(roleService.getProtectedRoles).toHaveBeenCalled(); }); it('should return the first name and last name when the user has no protected role', async () => { - // Arrange const user: UserDO = userDoFactory.withRoles([{ id: 'unprotectedId', name: RoleName.STUDENT }]).buildWithId({ lastName: 'lastName', firstName: 'firstName', }); - // Act const result: string = await service.getDisplayName(user); - // Assert expect(result).toEqual(`${user.firstName} ${user.lastName}`); expect(roleService.getProtectedRoles).toHaveBeenCalled(); }); diff --git a/apps/server/src/modules/user/service/user.service.ts b/apps/server/src/modules/user/service/user.service.ts index cc15404fc63..2cc95991f96 100644 --- a/apps/server/src/modules/user/service/user.service.ts +++ b/apps/server/src/modules/user/service/user.service.ts @@ -1,16 +1,16 @@ -import { ConfigService } from '@nestjs/config'; -import { EntityId, IFindOptions, LanguageType, User } from '@shared/domain'; -import { RoleReference, Page, UserDO } from '@shared/domain/domainobject'; -import { UserRepo } from '@shared/repo'; -import { UserDORepo } from '@shared/repo/user/user-do.repo'; import { AccountService } from '@modules/account'; import { AccountDto } from '@modules/account/services/dto'; -import { ICurrentUser } from '@modules/authentication'; // invalid import import { CurrentUserMapper } from '@modules/authentication/mapper'; import { RoleDto } from '@modules/role/service/dto/role.dto'; import { RoleService } from '@modules/role/service/role.service'; import { BadRequestException, Injectable } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; +import { EntityId, IFindOptions, LanguageType, User } from '@shared/domain'; +import { Page, RoleReference, UserDO } from '@shared/domain/domainobject'; +import { UserRepo } from '@shared/repo'; +import { UserDORepo } from '@shared/repo/user/user-do.repo'; +import { OauthCurrentUser } from '@modules/authentication/interface'; import { IUserConfig } from '../interfaces'; import { UserMapper } from '../mapper/user.mapper'; import { UserDto } from '../uc/dto/user.dto'; @@ -34,7 +34,7 @@ export class UserService { } /** - * @deprecated + * @deprecated use {@link UserService.findById} instead */ async getUser(id: string): Promise { const userEntity = await this.userRepo.findById(id, true); @@ -43,11 +43,11 @@ export class UserService { return userDto; } - async getResolvedUser(userId: EntityId): Promise { - const user: User = await this.userRepo.findById(userId, true); + async getResolvedUser(userId: EntityId): Promise { + const user: UserDO = await this.findById(userId); const account: AccountDto = await this.accountService.findByUserIdOrFail(userId); - const resolvedUser: ICurrentUser = CurrentUserMapper.userToICurrentUser(account.id, user, account.systemId); + const resolvedUser: OauthCurrentUser = CurrentUserMapper.mapToOauthCurrentUser(account.id, user, account.systemId); return resolvedUser; } diff --git a/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.spec.ts b/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.spec.ts index 4d15397548b..994c8042a6d 100644 --- a/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.spec.ts +++ b/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.spec.ts @@ -172,6 +172,7 @@ describe('VideoConferenceUc', () => { roles: [], schoolId: 'schoolId', accountId: 'accountId', + isExternalUser: false, }; defaultOptions = { everybodyJoinsAsModerator: false, diff --git a/apps/server/src/shared/testing/map-user-to-current-user.ts b/apps/server/src/shared/testing/map-user-to-current-user.ts index d835c822066..b8c975f125d 100644 --- a/apps/server/src/shared/testing/map-user-to-current-user.ts +++ b/apps/server/src/shared/testing/map-user-to-current-user.ts @@ -15,6 +15,7 @@ export const mapUserToCurrentUser = ( accountId: account ? account.id : new ObjectId().toHexString(), systemId, impersonated, + isExternalUser: false, }; return currentUser; From 46788a4dff6f1f7aa7ed377efe9b2cc8cc1f087c Mon Sep 17 00:00:00 2001 From: mamutmk5 <3045922+mamutmk5@users.noreply.github.com> Date: Thu, 2 Nov 2023 16:59:41 +0100 Subject: [PATCH 3/8] BC-5705 - Remove fwu ingress (#4515) --- .../schulcloud-server-core/tasks/main.yml | 8 ++-- .../templates/api-fwu-ingress.yml.j2 | 41 ------------------- 2 files changed, 5 insertions(+), 44 deletions(-) delete mode 100644 ansible/roles/schulcloud-server-core/templates/api-fwu-ingress.yml.j2 diff --git a/ansible/roles/schulcloud-server-core/tasks/main.yml b/ansible/roles/schulcloud-server-core/tasks/main.yml index 1b58c8a5413..64e257c5f59 100644 --- a/ansible/roles/schulcloud-server-core/tasks/main.yml +++ b/ansible/roles/schulcloud-server-core/tasks/main.yml @@ -92,12 +92,14 @@ template: api-fwu-deployment.yml.j2 when: FEATURE_FWU_CONTENT_ENABLED is defined and FEATURE_FWU_CONTENT_ENABLED|bool - - name: Fwu Learning Contents Ingress + - name: Fwu Learning Contents Ingress Remove kubernetes.core.k8s: kubeconfig: ~/.kube/config namespace: "{{ NAMESPACE }}" - template: api-fwu-ingress.yml.j2 - apply: yes + state: absent + api_version: networking.k8s.io/v1 + kind: Ingress + name: "{{ NAMESPACE }}-api-fwu-ingress" when: FEATURE_FWU_CONTENT_ENABLED is defined and FEATURE_FWU_CONTENT_ENABLED|bool - name: Delete Files CronJob diff --git a/ansible/roles/schulcloud-server-core/templates/api-fwu-ingress.yml.j2 b/ansible/roles/schulcloud-server-core/templates/api-fwu-ingress.yml.j2 deleted file mode 100644 index f42c322e45b..00000000000 --- a/ansible/roles/schulcloud-server-core/templates/api-fwu-ingress.yml.j2 +++ /dev/null @@ -1,41 +0,0 @@ -apiVersion: networking.k8s.io/v1 -kind: Ingress -metadata: - name: {{ NAMESPACE }}-api-fwu-ingress - namespace: {{ NAMESPACE }} - annotations: - nginx.ingress.kubernetes.io/ssl-redirect: "{{ TLS_ENABELD|default("false") }}" - nginx.ingress.kubernetes.io/proxy-body-size: "{{ INGRESS_MAX_BODY_SIZE|default("2560") }}m" - nginx.org/client-max-body-size: "{{ INGRESS_MAX_BODY_SIZE|default("2560") }}m" - # The following properties added with BC-3606. - # The header size of the request is too big. For e.g. state and the permanent growing jwt. - # Nginx throws away the Location header, resulting in the 502 Bad Gateway. - nginx.ingress.kubernetes.io/client-header-buffer-size: 100k - nginx.ingress.kubernetes.io/http2-max-header-size: 96k - nginx.ingress.kubernetes.io/large-client-header-buffers: 4 100k - nginx.ingress.kubernetes.io/proxy-buffer-size: 96k -{% if CLUSTER_ISSUER is defined %} - cert-manager.io/cluster-issuer: {{ CLUSTER_ISSUER }} -{% endif %} - -spec: - ingressClassName: nginx -{% if CLUSTER_ISSUER is defined or (TLS_ENABELD is defined and TLS_ENABELD|bool) %} - tls: - - hosts: - - {{ DOMAIN }} -{% if CLUSTER_ISSUER is defined %} - secretName: {{ DOMAIN }}-tls -{% endif %} -{% endif %} - rules: - - host: {{ DOMAIN }} - http: - paths: - - path: /api/v3/fwu/ - backend: - service: - name: api-fwu-svc - port: - number: {{ PORT_FWU_LEARNING_CONTENTS }} - pathType: Prefix From 0f9dee5a3196425623fc3b881c605e311df821ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= <103562092+MarvinOehlerkingCap@users.noreply.github.com> Date: Fri, 3 Nov 2023 07:21:07 +0100 Subject: [PATCH 4/8] N21-1296 Delete ContextExternalTool when deleting a ExternalToolElement on boards (#4507) --- apps/server/src/modules/board/board.module.ts | 10 +++--- .../modules/board/repo/board-do.repo.spec.ts | 4 ++- .../repo/recursive-delete.visitor.spec.ts | 31 ++++++++++++++++--- .../board/repo/recursive-delete.vistor.ts | 16 ++++++++-- .../src/modules/pseudonym/pseudonym.module.ts | 9 +++--- apps/server/src/modules/task/task.module.ts | 7 ++--- 6 files changed, 55 insertions(+), 22 deletions(-) diff --git a/apps/server/src/modules/board/board.module.ts b/apps/server/src/modules/board/board.module.ts index fb04364b6c3..09d00c46bbd 100644 --- a/apps/server/src/modules/board/board.module.ts +++ b/apps/server/src/modules/board/board.module.ts @@ -1,12 +1,12 @@ +import { FilesStorageClientModule } from '@modules/files-storage-client'; +import { ContextExternalToolModule } from '@modules/tool/context-external-tool'; +import { UserModule } from '@modules/user'; import { Module } from '@nestjs/common'; import { ContentElementFactory } from '@shared/domain'; import { ConsoleWriterModule } from '@shared/infra/console'; import { CourseRepo } from '@shared/repo'; import { LoggerModule } from '@src/core/logger'; -import { FilesStorageClientModule } from '../files-storage-client'; -import { UserModule } from '../user'; -import { BoardDoRepo, BoardNodeRepo } from './repo'; -import { RecursiveDeleteVisitor } from './repo/recursive-delete.vistor'; +import { BoardDoRepo, BoardNodeRepo, RecursiveDeleteVisitor } from './repo'; import { BoardDoAuthorizableService, BoardDoService, @@ -21,7 +21,7 @@ import { BoardDoCopyService, SchoolSpecificFileCopyServiceFactory } from './serv import { ColumnBoardCopyService } from './service/column-board-copy.service'; @Module({ - imports: [ConsoleWriterModule, FilesStorageClientModule, LoggerModule, UserModule], + imports: [ConsoleWriterModule, FilesStorageClientModule, LoggerModule, UserModule, ContextExternalToolModule], providers: [ BoardDoAuthorizableService, BoardDoRepo, diff --git a/apps/server/src/modules/board/repo/board-do.repo.spec.ts b/apps/server/src/modules/board/repo/board-do.repo.spec.ts index aa1c49224fe..3874e9301ba 100644 --- a/apps/server/src/modules/board/repo/board-do.repo.spec.ts +++ b/apps/server/src/modules/board/repo/board-do.repo.spec.ts @@ -1,6 +1,8 @@ import { createMock } from '@golevelup/ts-jest'; import { NotFoundError } from '@mikro-orm/core'; import { EntityManager } from '@mikro-orm/mongodb'; +import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; +import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; import { NotFoundException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { @@ -26,7 +28,6 @@ import { richTextElementFactory, richTextElementNodeFactory, } from '@shared/testing'; -import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; import { BoardDoRepo } from './board-do.repo'; import { BoardNodeRepo } from './board-node.repo'; import { RecursiveDeleteVisitor } from './recursive-delete.vistor'; @@ -46,6 +47,7 @@ describe(BoardDoRepo.name, () => { BoardNodeRepo, RecursiveDeleteVisitor, { provide: FilesStorageClientAdapterService, useValue: createMock() }, + { provide: ContextExternalToolService, useValue: createMock() }, ], }).compile(); repo = module.get(BoardDoRepo); diff --git a/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts b/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts index 9142cb33553..6236d5de8bb 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts @@ -1,10 +1,13 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { EntityManager } from '@mikro-orm/mongodb'; +import { FileDto, FilesStorageClientAdapterService } from '@modules/files-storage-client'; +import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; import { Test, TestingModule } from '@nestjs/testing'; import { FileRecordParentType } from '@shared/infra/rabbitmq'; import { columnBoardFactory, columnFactory, + contextExternalToolFactory, externalToolElementFactory, fileElementFactory, linkElementFactory, @@ -12,14 +15,15 @@ import { submissionContainerElementFactory, submissionItemFactory, } from '@shared/testing'; -import { FileDto, FilesStorageClientAdapterService } from '@modules/files-storage-client'; import { RecursiveDeleteVisitor } from './recursive-delete.vistor'; describe(RecursiveDeleteVisitor.name, () => { let module: TestingModule; + let service: RecursiveDeleteVisitor; + let em: DeepMocked; let filesStorageClientAdapterService: DeepMocked; - let service: RecursiveDeleteVisitor; + let contextExternalToolService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -27,12 +31,15 @@ describe(RecursiveDeleteVisitor.name, () => { RecursiveDeleteVisitor, { provide: EntityManager, useValue: createMock() }, { provide: FilesStorageClientAdapterService, useValue: createMock() }, + { provide: ContextExternalToolService, useValue: createMock() }, ], }).compile(); + service = module.get(RecursiveDeleteVisitor); em = module.get(EntityManager); filesStorageClientAdapterService = module.get(FilesStorageClientAdapterService); - service = module.get(RecursiveDeleteVisitor); + contextExternalToolService = module.get(ContextExternalToolService); + await setupEntities(); }); @@ -212,14 +219,30 @@ describe(RecursiveDeleteVisitor.name, () => { describe('visitExternalToolElementAsync', () => { const setup = () => { + const contextExternalTool = contextExternalToolFactory.buildWithId(); const childExternalToolElement = externalToolElementFactory.build(); const externalToolElement = externalToolElementFactory.build({ children: [childExternalToolElement], + contextExternalToolId: contextExternalTool.id, }); - return { externalToolElement, childExternalToolElement }; + contextExternalToolService.findById.mockResolvedValue(contextExternalTool); + + return { + externalToolElement, + childExternalToolElement, + contextExternalTool, + }; }; + it('should delete the context external tool that is linked to the element', async () => { + const { externalToolElement, contextExternalTool } = setup(); + + await service.visitExternalToolElementAsync(externalToolElement); + + expect(contextExternalToolService.deleteContextExternalTool).toHaveBeenCalledWith(contextExternalTool); + }); + it('should call entity remove', async () => { const { externalToolElement, childExternalToolElement } = setup(); diff --git a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts index 1c407391da4..0a1b08e663a 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts @@ -1,4 +1,7 @@ import { EntityManager } from '@mikro-orm/mongodb'; +import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; +import { ContextExternalTool } from '@modules/tool/context-external-tool/domain'; +import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; import { Injectable } from '@nestjs/common'; import { AnyBoardDo, @@ -14,13 +17,13 @@ import { SubmissionItem, } from '@shared/domain'; import { LinkElement } from '@shared/domain/domainobject/board/link-element.do'; -import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; @Injectable() export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { constructor( private readonly em: EntityManager, - private readonly filesStorageClientAdapterService: FilesStorageClientAdapterService + private readonly filesStorageClientAdapterService: FilesStorageClientAdapterService, + private readonly contextExternalToolService: ContextExternalToolService ) {} async visitColumnBoardAsync(columnBoard: ColumnBoard): Promise { @@ -67,7 +70,14 @@ export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { } async visitExternalToolElementAsync(externalToolElement: ExternalToolElement): Promise { - // TODO N21-1296: Delete linked ContextExternalTool + if (externalToolElement.contextExternalToolId) { + const linkedTool: ContextExternalTool = await this.contextExternalToolService.findById( + externalToolElement.contextExternalToolId + ); + + await this.contextExternalToolService.deleteContextExternalTool(linkedTool); + } + this.deleteNode(externalToolElement); await this.visitChildrenAsync(externalToolElement); diff --git a/apps/server/src/modules/pseudonym/pseudonym.module.ts b/apps/server/src/modules/pseudonym/pseudonym.module.ts index d282c5dd9fe..3a8bcdacbd1 100644 --- a/apps/server/src/modules/pseudonym/pseudonym.module.ts +++ b/apps/server/src/modules/pseudonym/pseudonym.module.ts @@ -1,14 +1,13 @@ -import { forwardRef, Module } from '@nestjs/common'; -import { LegacyLogger } from '@src/core/logger'; import { LearnroomModule } from '@modules/learnroom'; -import { UserModule } from '@modules/user'; import { ToolModule } from '@modules/tool'; -import { AuthorizationModule } from '@modules/authorization'; +import { UserModule } from '@modules/user'; +import { forwardRef, Module } from '@nestjs/common'; +import { LegacyLogger } from '@src/core/logger'; import { ExternalToolPseudonymRepo, PseudonymsRepo } from './repo'; import { FeathersRosterService, PseudonymService } from './service'; @Module({ - imports: [UserModule, LearnroomModule, forwardRef(() => ToolModule), forwardRef(() => AuthorizationModule)], + imports: [UserModule, LearnroomModule, forwardRef(() => ToolModule)], providers: [PseudonymService, PseudonymsRepo, ExternalToolPseudonymRepo, LegacyLogger, FeathersRosterService], exports: [PseudonymService, FeathersRosterService], }) diff --git a/apps/server/src/modules/task/task.module.ts b/apps/server/src/modules/task/task.module.ts index 696d608d0a3..45a0fdb720a 100644 --- a/apps/server/src/modules/task/task.module.ts +++ b/apps/server/src/modules/task/task.module.ts @@ -1,12 +1,11 @@ -import { forwardRef, Module } from '@nestjs/common'; -import { CourseRepo, LessonRepo, SubmissionRepo, TaskRepo } from '@shared/repo'; -import { AuthorizationModule } from '@modules/authorization'; import { CopyHelperModule } from '@modules/copy-helper'; import { FilesStorageClientModule } from '@modules/files-storage-client'; +import { Module } from '@nestjs/common'; +import { CourseRepo, LessonRepo, SubmissionRepo, TaskRepo } from '@shared/repo'; import { SubmissionService, TaskCopyService, TaskService } from './service'; @Module({ - imports: [forwardRef(() => AuthorizationModule), FilesStorageClientModule, CopyHelperModule], + imports: [FilesStorageClientModule, CopyHelperModule], providers: [TaskService, TaskCopyService, SubmissionService, TaskRepo, LessonRepo, CourseRepo, SubmissionRepo], exports: [TaskService, TaskCopyService, SubmissionService], }) From f4e73cd1520caf08beab6ec0b83ed06228982f8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= <103562092+MarvinOehlerkingCap@users.noreply.github.com> Date: Fri, 3 Nov 2023 07:43:55 +0100 Subject: [PATCH 5/8] N21-1397 Restrict access to combined class list for teachers (#4506) --- .../src/modules/class/domain/testing/index.ts | 1 + .../src/modules/class/entity/testing/index.ts | 1 + .../modules/class/repo/classes.repo.spec.ts | 38 ++- .../src/modules/class/repo/classes.repo.ts | 32 +- .../class/repo/mapper/class.mapper.spec.ts | 2 +- .../modules/class/repo/mapper/class.mapper.ts | 2 +- .../class/service/class.service.spec.ts | 29 +- .../modules/class/service/class.service.ts | 7 + .../controller/api-test/group.api.spec.ts | 29 +- .../group/controller/group.controller.ts | 8 +- .../mapper/group-response.mapper.ts | 2 +- .../modules/group/uc/dto/class-info.dto.ts | 4 +- .../src/modules/group/uc/group.uc.spec.ts | 300 ++++++++++++++++-- apps/server/src/modules/group/uc/group.uc.ts | 101 ++++-- .../group/uc/mapper/group-uc.mapper.ts | 6 +- .../domain/interface/permission.enum.ts | 1 + .../shared/testing/user-role-permissions.ts | 1 + backup/setup/migrations.json | 11 + backup/setup/roles.json | 6 +- ...5587322-add-group-full-admin-permission.js | 74 +++++ 20 files changed, 554 insertions(+), 101 deletions(-) create mode 100644 apps/server/src/modules/class/domain/testing/index.ts create mode 100644 apps/server/src/modules/class/entity/testing/index.ts create mode 100644 migrations/1698325587322-add-group-full-admin-permission.js diff --git a/apps/server/src/modules/class/domain/testing/index.ts b/apps/server/src/modules/class/domain/testing/index.ts new file mode 100644 index 00000000000..3c5809ece1b --- /dev/null +++ b/apps/server/src/modules/class/domain/testing/index.ts @@ -0,0 +1 @@ +export * from './factory/class.factory'; diff --git a/apps/server/src/modules/class/entity/testing/index.ts b/apps/server/src/modules/class/entity/testing/index.ts new file mode 100644 index 00000000000..45893909755 --- /dev/null +++ b/apps/server/src/modules/class/entity/testing/index.ts @@ -0,0 +1 @@ +export * from './factory/class.entity.factory'; diff --git a/apps/server/src/modules/class/repo/classes.repo.spec.ts b/apps/server/src/modules/class/repo/classes.repo.spec.ts index 302a8f2de8e..7801045aff0 100644 --- a/apps/server/src/modules/class/repo/classes.repo.spec.ts +++ b/apps/server/src/modules/class/repo/classes.repo.spec.ts @@ -1,10 +1,11 @@ import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; +import { classEntityFactory } from '@modules/class/entity/testing/factory/class.entity.factory'; import { Test } from '@nestjs/testing'; import { TestingModule } from '@nestjs/testing/testing-module'; +import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { SchoolEntity } from '@shared/domain'; import { MongoMemoryDatabaseModule } from '@shared/infra/database'; import { cleanupCollections, schoolFactory } from '@shared/testing'; -import { classEntityFactory } from '@modules/class/entity/testing/factory/class.entity.factory'; import { Class } from '../domain'; import { ClassEntity } from '../entity'; import { ClassesRepo } from './classes.repo'; @@ -48,6 +49,7 @@ describe(ClassesRepo.name, () => { const classes: ClassEntity[] = classEntityFactory.buildListWithId(3, { schoolId: school.id }); await em.persistAndFlush(classes); + em.clear(); return { school, @@ -78,9 +80,13 @@ describe(ClassesRepo.name, () => { const setup = async () => { const testUser = new ObjectId(); const class1: ClassEntity = classEntityFactory.withUserIds([testUser, new ObjectId()]).buildWithId(); - const class2: ClassEntity = classEntityFactory.withUserIds([testUser, new ObjectId()]).buildWithId(); + const class2: ClassEntity = classEntityFactory + .withUserIds([new ObjectId()]) + .buildWithId({ teacherIds: [testUser] }); const class3: ClassEntity = classEntityFactory.withUserIds([new ObjectId(), new ObjectId()]).buildWithId(); + await em.persistAndFlush([class1, class2, class3]); + em.clear(); return { class1, @@ -104,7 +110,7 @@ describe(ClassesRepo.name, () => { }); describe('updateMany', () => { - describe('When deleting user data from classes', () => { + describe('when deleting user data from classes', () => { const setup = async () => { const testUser1 = new ObjectId(); const testUser2 = new ObjectId(); @@ -112,7 +118,9 @@ describe(ClassesRepo.name, () => { const class1: ClassEntity = classEntityFactory.withUserIds([testUser1, testUser2]).buildWithId(); const class2: ClassEntity = classEntityFactory.withUserIds([testUser1, testUser3]).buildWithId(); const class3: ClassEntity = classEntityFactory.withUserIds([testUser2, testUser3]).buildWithId(); + await em.persistAndFlush([class1, class2, class3]); + em.clear(); return { class1, @@ -144,5 +152,29 @@ describe(ClassesRepo.name, () => { expect(result3).toHaveLength(2); }); }); + + describe('when updating a class that does not exist', () => { + const setup = async () => { + const class1: ClassEntity = classEntityFactory.buildWithId(); + const class2: ClassEntity = classEntityFactory.buildWithId(); + + await em.persistAndFlush([class1]); + em.clear(); + + return { + class1, + class2, + }; + }; + + it('should throw an error', async () => { + const { class1, class2 } = await setup(); + + const updatedArray: ClassEntity[] = [class1, class2]; + const domainObjectsArray: Class[] = ClassMapper.mapToDOs(updatedArray); + + await expect(repo.updateMany(domainObjectsArray)).rejects.toThrow(NotFoundLoggableException); + }); + }); }); }); diff --git a/apps/server/src/modules/class/repo/classes.repo.ts b/apps/server/src/modules/class/repo/classes.repo.ts index 378b3de9716..24300bbe673 100644 --- a/apps/server/src/modules/class/repo/classes.repo.ts +++ b/apps/server/src/modules/class/repo/classes.repo.ts @@ -1,5 +1,6 @@ import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; import { Injectable } from '@nestjs/common'; +import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { EntityId } from '@shared/domain'; import { Class } from '../domain'; import { ClassEntity } from '../entity'; @@ -18,7 +19,9 @@ export class ClassesRepo { } async findAllByUserId(userId: EntityId): Promise { - const classes: ClassEntity[] = await this.em.find(ClassEntity, { userIds: new ObjectId(userId) }); + const classes: ClassEntity[] = await this.em.find(ClassEntity, { + $or: [{ userIds: new ObjectId(userId) }, { teacherIds: new ObjectId(userId) }], + }); const mapped: Class[] = ClassMapper.mapToDOs(classes); @@ -26,9 +29,30 @@ export class ClassesRepo { } async updateMany(classes: Class[]): Promise { - const classesEntities = ClassMapper.mapToEntities(classes); - const referencedEntities = classesEntities.map((classEntity) => this.em.getReference(ClassEntity, classEntity.id)); + const classMap: Map = new Map( + classes.map((clazz: Class): [string, Class] => [clazz.id, clazz]) + ); - await this.em.persistAndFlush(referencedEntities); + const existingEntities: ClassEntity[] = await this.em.find(ClassEntity, { + id: { $in: Array.from(classMap.keys()) }, + }); + + if (existingEntities.length < classes.length) { + const missingEntityIds: string[] = Array.from(classMap.keys()).filter( + (classId) => !existingEntities.find((entity) => entity.id === classId) + ); + + throw new NotFoundLoggableException(Class.name, 'id', missingEntityIds.toString()); + } + + existingEntities.forEach((entity) => { + const updatedDomainObject: Class | undefined = classMap.get(entity.id); + + const updatedEntity: ClassEntity = ClassMapper.mapToEntity(updatedDomainObject as Class); + + this.em.assign(entity, updatedEntity); + }); + + await this.em.persistAndFlush(existingEntities); } } diff --git a/apps/server/src/modules/class/repo/mapper/class.mapper.spec.ts b/apps/server/src/modules/class/repo/mapper/class.mapper.spec.ts index 53d0ecd4360..f1e71da5c4d 100644 --- a/apps/server/src/modules/class/repo/mapper/class.mapper.spec.ts +++ b/apps/server/src/modules/class/repo/mapper/class.mapper.spec.ts @@ -3,7 +3,7 @@ import { Class } from '../../domain'; import { ClassSourceOptions } from '../../domain/class-source-options.do'; import { classFactory } from '../../domain/testing/factory/class.factory'; import { ClassEntity } from '../../entity'; -import { classEntityFactory } from '../../entity/testing/factory/class.entity.factory'; +import { classEntityFactory } from '../../entity/testing'; import { ClassMapper } from './class.mapper'; describe(ClassMapper.name, () => { diff --git a/apps/server/src/modules/class/repo/mapper/class.mapper.ts b/apps/server/src/modules/class/repo/mapper/class.mapper.ts index 6340ffce7b0..8ae5e3b79b9 100644 --- a/apps/server/src/modules/class/repo/mapper/class.mapper.ts +++ b/apps/server/src/modules/class/repo/mapper/class.mapper.ts @@ -23,7 +23,7 @@ export class ClassMapper { }); } - private static mapToEntity(domainObject: Class): ClassEntity { + static mapToEntity(domainObject: Class): ClassEntity { return new ClassEntity({ id: domainObject.id, name: domainObject.name, diff --git a/apps/server/src/modules/class/service/class.service.spec.ts b/apps/server/src/modules/class/service/class.service.spec.ts index 850eaf655a6..5ba4b367b59 100644 --- a/apps/server/src/modules/class/service/class.service.spec.ts +++ b/apps/server/src/modules/class/service/class.service.spec.ts @@ -4,9 +4,9 @@ import { InternalServerErrorException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { EntityId } from '@shared/domain'; import { setupEntities } from '@shared/testing'; -import { classEntityFactory } from '@modules/class/entity/testing/factory/class.entity.factory'; import { Class } from '../domain'; -import { classFactory } from '../domain/testing/factory/class.factory'; +import { classFactory } from '../domain/testing'; +import { classEntityFactory } from '../entity/testing'; import { ClassesRepo } from '../repo'; import { ClassMapper } from '../repo/mapper'; import { ClassService } from './class.service'; @@ -74,6 +74,31 @@ describe(ClassService.name, () => { }); }); + describe('findAllByUserId', () => { + describe('when the user has classes', () => { + const setup = () => { + const userId: string = new ObjectId().toHexString(); + + const classes: Class[] = classFactory.buildList(3); + + classesRepo.findAllByUserId.mockResolvedValueOnce(classes); + + return { + userId, + classes, + }; + }; + + it('should return the classes', async () => { + const { userId, classes } = setup(); + + const result: Class[] = await service.findAllByUserId(userId); + + expect(result).toEqual(classes); + }); + }); + }); + describe('deleteUserDataFromClasses', () => { describe('when user is missing', () => { const setup = () => { diff --git a/apps/server/src/modules/class/service/class.service.ts b/apps/server/src/modules/class/service/class.service.ts index 9671c456912..772b9f0c4d4 100644 --- a/apps/server/src/modules/class/service/class.service.ts +++ b/apps/server/src/modules/class/service/class.service.ts @@ -13,6 +13,13 @@ export class ClassService { return classes; } + public async findAllByUserId(userId: EntityId): Promise { + const classes: Class[] = await this.classesRepo.findAllByUserId(userId); + + return classes; + } + + // FIXME There is no usage of this method public async deleteUserDataFromClasses(userId: EntityId): Promise { if (!userId) { throw new InternalServerErrorException('User id is missing'); diff --git a/apps/server/src/modules/group/controller/api-test/group.api.spec.ts b/apps/server/src/modules/group/controller/api-test/group.api.spec.ts index 34a49c03a35..2d9f4105f80 100644 --- a/apps/server/src/modules/group/controller/api-test/group.api.spec.ts +++ b/apps/server/src/modules/group/controller/api-test/group.api.spec.ts @@ -1,4 +1,7 @@ import { EntityManager } from '@mikro-orm/mongodb'; +import { ClassEntity } from '@modules/class/entity'; +import { classEntityFactory } from '@modules/class/entity/testing'; +import { ServerTestModule } from '@modules/server'; import { HttpStatus, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { Role, RoleName, SchoolEntity, SchoolYearEntity, SortOrder, SystemEntity, User } from '@shared/domain'; @@ -12,9 +15,6 @@ import { UserAndAccountTestFactory, userFactory, } from '@shared/testing'; -import { ClassEntity } from '@modules/class/entity'; -import { classEntityFactory } from '@modules/class/entity/testing/factory/class.entity.factory'; -import { ServerTestModule } from '@modules/server'; import { ObjectId } from 'bson'; import { GroupEntity, GroupEntityTypes } from '../../entity'; import { ClassRootType } from '../../uc/dto/class-root-type'; @@ -135,29 +135,6 @@ describe('Group (API)', () => { }); }); }); - - describe('when an invalid user requests a list of classes', () => { - const setup = async () => { - const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent(); - - await em.persistAndFlush([studentAccount, studentUser]); - em.clear(); - - const studentClient = await testApiClient.login(studentAccount); - - return { - studentClient, - }; - }; - - it('should return forbidden', async () => { - const { studentClient } = await setup(); - - const response = await studentClient.get(`/class`); - - expect(response.status).toEqual(HttpStatus.FORBIDDEN); - }); - }); }); describe('[GET] /groups/:groupId', () => { diff --git a/apps/server/src/modules/group/controller/group.controller.ts b/apps/server/src/modules/group/controller/group.controller.ts index 9e5f4b3b51a..a7dc0c77563 100644 --- a/apps/server/src/modules/group/controller/group.controller.ts +++ b/apps/server/src/modules/group/controller/group.controller.ts @@ -1,9 +1,9 @@ +import { Authenticate, CurrentUser, ICurrentUser } from '@modules/authentication'; import { Controller, Get, HttpStatus, Param, Query } from '@nestjs/common'; import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; import { PaginationParams } from '@shared/controller'; import { Page } from '@shared/domain'; import { ErrorResponse } from '@src/core/error/dto'; -import { ICurrentUser, Authenticate, CurrentUser } from '@modules/authentication'; import { GroupUc } from '../uc'; import { ClassInfoDto, ResolvedGroupDto } from '../uc/dto'; import { ClassInfoSearchListResponse, ClassSortParams, GroupIdParams, GroupResponse } from './dto'; @@ -15,17 +15,17 @@ import { GroupResponseMapper } from './mapper'; export class GroupController { constructor(private readonly groupUc: GroupUc) {} - @ApiOperation({ summary: 'Get a list of classes and groups of type class for the current users school.' }) + @ApiOperation({ summary: 'Get a list of classes and groups of type class for the current user.' }) @ApiResponse({ status: HttpStatus.OK, type: ClassInfoSearchListResponse }) @ApiResponse({ status: '4XX', type: ErrorResponse }) @ApiResponse({ status: '5XX', type: ErrorResponse }) @Get('/class') - public async findClassesForSchool( + public async findClasses( @Query() pagination: PaginationParams, @Query() sortingQuery: ClassSortParams, @CurrentUser() currentUser: ICurrentUser ): Promise { - const board: Page = await this.groupUc.findAllClassesForSchool( + const board: Page = await this.groupUc.findAllClasses( currentUser.userId, currentUser.schoolId, pagination.skip, diff --git a/apps/server/src/modules/group/controller/mapper/group-response.mapper.ts b/apps/server/src/modules/group/controller/mapper/group-response.mapper.ts index 6efd02d899d..8c990cbd44a 100644 --- a/apps/server/src/modules/group/controller/mapper/group-response.mapper.ts +++ b/apps/server/src/modules/group/controller/mapper/group-response.mapper.ts @@ -40,7 +40,7 @@ export class GroupResponseMapper { type: classInfo.type, name: classInfo.name, externalSourceName: classInfo.externalSourceName, - teachers: classInfo.teachers, + teachers: classInfo.teacherNames, schoolYear: classInfo.schoolYear, isUpgradable: classInfo.isUpgradable, }); diff --git a/apps/server/src/modules/group/uc/dto/class-info.dto.ts b/apps/server/src/modules/group/uc/dto/class-info.dto.ts index 8c564d9e106..611275e3bcd 100644 --- a/apps/server/src/modules/group/uc/dto/class-info.dto.ts +++ b/apps/server/src/modules/group/uc/dto/class-info.dto.ts @@ -9,7 +9,7 @@ export class ClassInfoDto { externalSourceName?: string; - teachers: string[]; + teacherNames: string[]; schoolYear?: string; @@ -20,7 +20,7 @@ export class ClassInfoDto { this.type = props.type; this.name = props.name; this.externalSourceName = props.externalSourceName; - this.teachers = props.teachers; + this.teacherNames = props.teacherNames; this.schoolYear = props.schoolYear; this.isUpgradable = props.isUpgradable; } diff --git a/apps/server/src/modules/group/uc/group.uc.spec.ts b/apps/server/src/modules/group/uc/group.uc.spec.ts index 34cb55a1354..d5236826def 100644 --- a/apps/server/src/modules/group/uc/group.uc.spec.ts +++ b/apps/server/src/modules/group/uc/group.uc.spec.ts @@ -1,5 +1,14 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; +import { Action, AuthorizationContext, AuthorizationService } from '@modules/authorization'; +import { ClassService } from '@modules/class'; +import { Class } from '@modules/class/domain'; +import { classFactory } from '@modules/class/domain/testing/factory/class.factory'; +import { LegacySchoolService, SchoolYearService } from '@modules/legacy-school'; +import { RoleService } from '@modules/role'; +import { RoleDto } from '@modules/role/service/dto/role.dto'; +import { SystemDto, SystemService } from '@modules/system'; +import { UserService } from '@modules/user'; import { ForbiddenException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { NotFoundLoggableException } from '@shared/common/loggable-exception'; @@ -14,15 +23,6 @@ import { userDoFactory, userFactory, } from '@shared/testing'; -import { Action, AuthorizationContext, AuthorizationService } from '@modules/authorization'; -import { ClassService } from '@modules/class'; -import { Class } from '@modules/class/domain'; -import { classFactory } from '@modules/class/domain/testing/factory/class.factory'; -import { LegacySchoolService, SchoolYearService } from '@modules/legacy-school'; -import { RoleService } from '@modules/role'; -import { RoleDto } from '@modules/role/service/dto/role.dto'; -import { SystemDto, SystemService } from '@modules/system'; -import { UserService } from '@modules/user'; import { Group, GroupTypes } from '../domain'; import { GroupService } from '../service'; import { ClassInfoDto, ResolvedGroupDto } from './dto'; @@ -102,7 +102,7 @@ describe('GroupUc', () => { jest.resetAllMocks(); }); - describe('findClassesForSchool', () => { + describe('findAllClasses', () => { describe('when the user has no permission', () => { const setup = () => { const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); @@ -114,6 +114,7 @@ describe('GroupUc', () => { authorizationService.checkPermission.mockImplementation(() => { throw error; }); + authorizationService.hasAllPermissions.mockReturnValueOnce(false); return { user, @@ -124,13 +125,13 @@ describe('GroupUc', () => { it('should throw forbidden', async () => { const { user, error } = setup(); - const func = () => uc.findAllClassesForSchool(user.id, user.school.id); + const func = () => uc.findAllClasses(user.id, user.school.id); await expect(func).rejects.toThrow(error); }); }); - describe('when the school has classes', () => { + describe('when accessing as a normal user', () => { const setup = () => { const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); const { studentUser } = UserAndAccountTestFactory.buildStudent(); @@ -181,8 +182,9 @@ describe('GroupUc', () => { schoolService.getSchoolById.mockResolvedValueOnce(school); authorizationService.getUserWithPermissions.mockResolvedValueOnce(teacherUser); - classService.findClassesForSchool.mockResolvedValueOnce([clazz]); - groupService.findClassesForSchool.mockResolvedValueOnce([group, groupWithSystem]); + authorizationService.hasAllPermissions.mockReturnValueOnce(false); + classService.findAllByUserId.mockResolvedValueOnce([clazz]); + groupService.findByUser.mockResolvedValueOnce([group, groupWithSystem]); systemService.findById.mockResolvedValue(system); userService.findById.mockImplementation((userId: string): Promise => { if (userId === teacherUser.id) { @@ -222,23 +224,34 @@ describe('GroupUc', () => { it('should check the required permissions', async () => { const { teacherUser, school } = setup(); - await uc.findAllClassesForSchool(teacherUser.id, teacherUser.school.id); + await uc.findAllClasses(teacherUser.id, teacherUser.school.id); expect(authorizationService.checkPermission).toHaveBeenCalledWith<[User, LegacySchoolDo, AuthorizationContext]>( teacherUser, school, { action: Action.read, - requiredPermissions: [Permission.CLASS_LIST, Permission.GROUP_LIST], + requiredPermissions: [Permission.CLASS_VIEW, Permission.GROUP_VIEW], } ); }); + it('should check the access to the full list', async () => { + const { teacherUser } = setup(); + + await uc.findAllClasses(teacherUser.id, teacherUser.school.id); + + expect(authorizationService.hasAllPermissions).toHaveBeenCalledWith<[User, string[]]>(teacherUser, [ + Permission.CLASS_FULL_ADMIN, + Permission.GROUP_FULL_ADMIN, + ]); + }); + describe('when no pagination is given', () => { it('should return all classes sorted by name', async () => { const { teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); - const result: Page = await uc.findAllClassesForSchool(teacherUser.id, teacherUser.school.id); + const result: Page = await uc.findAllClasses(teacherUser.id, teacherUser.school.id); expect(result).toEqual>({ data: [ @@ -247,7 +260,7 @@ describe('GroupUc', () => { name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, type: ClassRootType.CLASS, externalSourceName: clazz.source, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], schoolYear: schoolYear.name, isUpgradable: false, }, @@ -255,14 +268,14 @@ describe('GroupUc', () => { id: group.id, name: group.name, type: ClassRootType.GROUP, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], }, { id: groupWithSystem.id, name: groupWithSystem.name, type: ClassRootType.GROUP, externalSourceName: system.displayName, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], }, ], total: 3, @@ -274,7 +287,7 @@ describe('GroupUc', () => { it('should return all classes sorted by external source name in descending order', async () => { const { teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); - const result: Page = await uc.findAllClassesForSchool( + const result: Page = await uc.findAllClasses( teacherUser.id, teacherUser.school.id, undefined, @@ -290,7 +303,7 @@ describe('GroupUc', () => { name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, type: ClassRootType.CLASS, externalSourceName: clazz.source, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], schoolYear: schoolYear.name, isUpgradable: false, }, @@ -299,13 +312,13 @@ describe('GroupUc', () => { name: groupWithSystem.name, type: ClassRootType.GROUP, externalSourceName: system.displayName, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], }, { id: group.id, name: group.name, type: ClassRootType.GROUP, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], }, ], total: 3, @@ -317,7 +330,7 @@ describe('GroupUc', () => { it('should return the selected page', async () => { const { teacherUser, group } = setup(); - const result: Page = await uc.findAllClassesForSchool( + const result: Page = await uc.findAllClasses( teacherUser.id, teacherUser.school.id, 1, @@ -332,7 +345,242 @@ describe('GroupUc', () => { id: group.id, name: group.name, type: ClassRootType.GROUP, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], + }, + ], + total: 3, + }); + }); + }); + }); + + describe('when accessing as a user with elevated permission', () => { + const setup = () => { + const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); + const { studentUser } = UserAndAccountTestFactory.buildStudent(); + const { teacherUser } = UserAndAccountTestFactory.buildTeacher(); + const { adminUser } = UserAndAccountTestFactory.buildAdmin(); + const teacherRole: RoleDto = roleDtoFactory.buildWithId({ + id: teacherUser.roles[0].id, + name: teacherUser.roles[0].name, + }); + const studentRole: RoleDto = roleDtoFactory.buildWithId({ + id: studentUser.roles[0].id, + name: studentUser.roles[0].name, + }); + const adminUserDo: UserDO = userDoFactory.buildWithId({ + id: adminUser.id, + lastName: adminUser.lastName, + roles: [{ id: adminUser.roles[0].id, name: adminUser.roles[0].name }], + }); + const teacherUserDo: UserDO = userDoFactory.buildWithId({ + id: teacherUser.id, + lastName: teacherUser.lastName, + roles: [{ id: teacherUser.roles[0].id, name: teacherUser.roles[0].name }], + }); + const studentUserDo: UserDO = userDoFactory.buildWithId({ + id: studentUser.id, + lastName: studentUser.lastName, + roles: [{ id: studentUser.roles[0].id, name: studentUser.roles[0].name }], + }); + const schoolYear: SchoolYearEntity = schoolYearFactory.buildWithId(); + const clazz: Class = classFactory.build({ + name: 'A', + teacherIds: [teacherUser.id], + source: 'LDAP', + year: schoolYear.id, + }); + const system: SystemDto = new SystemDto({ + id: new ObjectId().toHexString(), + displayName: 'External System', + type: 'oauth2', + }); + const group: Group = groupFactory.build({ + name: 'B', + users: [{ userId: teacherUser.id, roleId: teacherUser.roles[0].id }], + externalSource: undefined, + }); + const groupWithSystem: Group = groupFactory.build({ + name: 'C', + externalSource: { externalId: 'externalId', systemId: system.id }, + users: [ + { userId: teacherUser.id, roleId: teacherUser.roles[0].id }, + { userId: studentUser.id, roleId: studentUser.roles[0].id }, + ], + }); + + schoolService.getSchoolById.mockResolvedValueOnce(school); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(adminUser); + authorizationService.hasAllPermissions.mockReturnValueOnce(true); + classService.findClassesForSchool.mockResolvedValueOnce([clazz]); + groupService.findClassesForSchool.mockResolvedValueOnce([group, groupWithSystem]); + systemService.findById.mockResolvedValue(system); + + userService.findById.mockImplementation((userId: string): Promise => { + if (userId === teacherUser.id) { + return Promise.resolve(teacherUserDo); + } + + if (userId === studentUser.id) { + return Promise.resolve(studentUserDo); + } + + if (userId === adminUser.id) { + return Promise.resolve(adminUserDo); + } + + throw new Error(); + }); + roleService.findById.mockImplementation((roleId: string): Promise => { + if (roleId === teacherUser.roles[0].id) { + return Promise.resolve(teacherRole); + } + + if (roleId === studentUser.roles[0].id) { + return Promise.resolve(studentRole); + } + + throw new Error(); + }); + schoolYearService.findById.mockResolvedValue(schoolYear); + + return { + adminUser, + teacherUser, + school, + clazz, + group, + groupWithSystem, + system, + schoolYear, + }; + }; + + it('should check the required permissions', async () => { + const { adminUser, school } = setup(); + + await uc.findAllClasses(adminUser.id, adminUser.school.id); + + expect(authorizationService.checkPermission).toHaveBeenCalledWith<[User, LegacySchoolDo, AuthorizationContext]>( + adminUser, + school, + { + action: Action.read, + requiredPermissions: [Permission.CLASS_VIEW, Permission.GROUP_VIEW], + } + ); + }); + + it('should check the access to the full list', async () => { + const { adminUser } = setup(); + + await uc.findAllClasses(adminUser.id, adminUser.school.id); + + expect(authorizationService.hasAllPermissions).toHaveBeenCalledWith<[User, string[]]>(adminUser, [ + Permission.CLASS_FULL_ADMIN, + Permission.GROUP_FULL_ADMIN, + ]); + }); + + describe('when no pagination is given', () => { + it('should return all classes sorted by name', async () => { + const { adminUser, teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); + + const result: Page = await uc.findAllClasses(adminUser.id, adminUser.school.id); + + expect(result).toEqual>({ + data: [ + { + id: clazz.id, + name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, + type: ClassRootType.CLASS, + externalSourceName: clazz.source, + teacherNames: [teacherUser.lastName], + schoolYear: schoolYear.name, + isUpgradable: false, + }, + { + id: group.id, + name: group.name, + type: ClassRootType.GROUP, + teacherNames: [teacherUser.lastName], + }, + { + id: groupWithSystem.id, + name: groupWithSystem.name, + type: ClassRootType.GROUP, + externalSourceName: system.displayName, + teacherNames: [teacherUser.lastName], + }, + ], + total: 3, + }); + }); + }); + + describe('when sorting by external source name in descending order', () => { + it('should return all classes sorted by external source name in descending order', async () => { + const { adminUser, teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); + + const result: Page = await uc.findAllClasses( + adminUser.id, + adminUser.school.id, + undefined, + undefined, + 'externalSourceName', + SortOrder.desc + ); + + expect(result).toEqual>({ + data: [ + { + id: clazz.id, + name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, + type: ClassRootType.CLASS, + externalSourceName: clazz.source, + teacherNames: [teacherUser.lastName], + schoolYear: schoolYear.name, + isUpgradable: false, + }, + { + id: groupWithSystem.id, + name: groupWithSystem.name, + type: ClassRootType.GROUP, + externalSourceName: system.displayName, + teacherNames: [teacherUser.lastName], + }, + { + id: group.id, + name: group.name, + type: ClassRootType.GROUP, + teacherNames: [teacherUser.lastName], + }, + ], + total: 3, + }); + }); + }); + + describe('when using pagination', () => { + it('should return the selected page', async () => { + const { adminUser, teacherUser, group } = setup(); + + const result: Page = await uc.findAllClasses( + adminUser.id, + adminUser.school.id, + 1, + 1, + 'name', + SortOrder.asc + ); + + expect(result).toEqual>({ + data: [ + { + id: group.id, + name: group.name, + type: ClassRootType.GROUP, + teacherNames: [teacherUser.lastName], }, ], total: 3, diff --git a/apps/server/src/modules/group/uc/group.uc.ts b/apps/server/src/modules/group/uc/group.uc.ts index 2421e444e73..f40750fc852 100644 --- a/apps/server/src/modules/group/uc/group.uc.ts +++ b/apps/server/src/modules/group/uc/group.uc.ts @@ -1,5 +1,3 @@ -import { Injectable } from '@nestjs/common'; -import { EntityId, LegacySchoolDo, Page, Permission, SchoolYearEntity, SortOrder, User, UserDO } from '@shared/domain'; import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; import { ClassService } from '@modules/class'; import { Class } from '@modules/class/domain'; @@ -8,6 +6,8 @@ import { RoleService } from '@modules/role'; import { RoleDto } from '@modules/role/service/dto/role.dto'; import { SystemDto, SystemService } from '@modules/system'; import { UserService } from '@modules/user'; +import { Injectable } from '@nestjs/common'; +import { EntityId, LegacySchoolDo, Page, Permission, SchoolYearEntity, SortOrder, User, UserDO } from '@shared/domain'; import { Group, GroupUser } from '../domain'; import { GroupService } from '../service'; import { SortHelper } from '../util'; @@ -27,7 +27,7 @@ export class GroupUc { private readonly schoolYearService: SchoolYearService ) {} - public async findAllClassesForSchool( + public async findAllClasses( userId: EntityId, schoolId: EntityId, skip = 0, @@ -41,10 +41,20 @@ export class GroupUc { this.authorizationService.checkPermission( user, school, - AuthorizationContextBuilder.read([Permission.CLASS_LIST, Permission.GROUP_LIST]) + AuthorizationContextBuilder.read([Permission.CLASS_VIEW, Permission.GROUP_VIEW]) ); - const combinedClassInfo: ClassInfoDto[] = await this.findCombinedClassListForSchool(schoolId); + const canSeeFullList: boolean = this.authorizationService.hasAllPermissions(user, [ + Permission.CLASS_FULL_ADMIN, + Permission.GROUP_FULL_ADMIN, + ]); + + let combinedClassInfo: ClassInfoDto[]; + if (canSeeFullList) { + combinedClassInfo = await this.findCombinedClassListForSchool(schoolId); + } else { + combinedClassInfo = await this.findCombinedClassListForUser(userId); + } combinedClassInfo.sort((a: ClassInfoDto, b: ClassInfoDto): number => SortHelper.genericSortFunction(a[sortBy], b[sortBy], sortOrder) @@ -57,7 +67,7 @@ export class GroupUc { return page; } - private async findCombinedClassListForSchool(schoolId: string): Promise { + private async findCombinedClassListForSchool(schoolId: EntityId): Promise { const [classInfosFromClasses, classInfosFromGroups] = await Promise.all([ await this.findClassesForSchool(schoolId), await this.findGroupsOfTypeClassForSchool(schoolId), @@ -68,52 +78,91 @@ export class GroupUc { return combinedClassInfo; } + private async findCombinedClassListForUser(userId: EntityId): Promise { + const [classInfosFromClasses, classInfosFromGroups] = await Promise.all([ + await this.findClassesForUser(userId), + await this.findGroupsOfTypeClassForUser(userId), + ]); + + const combinedClassInfo: ClassInfoDto[] = [...classInfosFromClasses, ...classInfosFromGroups]; + + return combinedClassInfo; + } + private async findClassesForSchool(schoolId: EntityId): Promise { const classes: Class[] = await this.classService.findClassesForSchool(schoolId); const classInfosFromClasses: ClassInfoDto[] = await Promise.all( - classes.map(async (clazz: Class): Promise => { - const teachers: UserDO[] = await Promise.all( - clazz.teacherIds.map((teacherId: EntityId) => this.userService.findById(teacherId)) - ); + classes.map((clazz) => this.getClassInfoFromClass(clazz)) + ); - let schoolYear: SchoolYearEntity | undefined; - if (clazz.year) { - schoolYear = await this.schoolYearService.findById(clazz.year); - } + return classInfosFromClasses; + } - const mapped: ClassInfoDto = GroupUcMapper.mapClassToClassInfoDto(clazz, teachers, schoolYear); + private async findClassesForUser(userId: EntityId): Promise { + const classes: Class[] = await this.classService.findAllByUserId(userId); - return mapped; - }) + const classInfosFromClasses: ClassInfoDto[] = await Promise.all( + classes.map((clazz) => this.getClassInfoFromClass(clazz)) ); return classInfosFromClasses; } + private async getClassInfoFromClass(clazz: Class): Promise { + const teachers: UserDO[] = await Promise.all( + clazz.teacherIds.map((teacherId: EntityId) => this.userService.findById(teacherId)) + ); + + let schoolYear: SchoolYearEntity | undefined; + if (clazz.year) { + schoolYear = await this.schoolYearService.findById(clazz.year); + } + + const mapped: ClassInfoDto = GroupUcMapper.mapClassToClassInfoDto(clazz, teachers, schoolYear); + + return mapped; + } + private async findGroupsOfTypeClassForSchool(schoolId: EntityId): Promise { const groupsOfTypeClass: Group[] = await this.groupService.findClassesForSchool(schoolId); const systemMap: Map = await this.findSystemNamesForGroups(groupsOfTypeClass); const classInfosFromGroups: ClassInfoDto[] = await Promise.all( - groupsOfTypeClass.map(async (group: Group): Promise => { - let system: SystemDto | undefined; - if (group.externalSource) { - system = systemMap.get(group.externalSource.systemId); - } + groupsOfTypeClass.map(async (group: Group): Promise => this.getClassInfoFromGroup(group, systemMap)) + ); - const resolvedUsers: ResolvedGroupUser[] = await this.findUsersForGroup(group); + return classInfosFromGroups; + } - const mapped: ClassInfoDto = GroupUcMapper.mapGroupToClassInfoDto(group, resolvedUsers, system); + private async findGroupsOfTypeClassForUser(userId: EntityId): Promise { + const user: UserDO = await this.userService.findById(userId); - return mapped; - }) + const groupsOfTypeClass: Group[] = await this.groupService.findByUser(user); + + const systemMap: Map = await this.findSystemNamesForGroups(groupsOfTypeClass); + + const classInfosFromGroups: ClassInfoDto[] = await Promise.all( + groupsOfTypeClass.map(async (group: Group): Promise => this.getClassInfoFromGroup(group, systemMap)) ); return classInfosFromGroups; } + private async getClassInfoFromGroup(group: Group, systemMap: Map): Promise { + let system: SystemDto | undefined; + if (group.externalSource) { + system = systemMap.get(group.externalSource.systemId); + } + + const resolvedUsers: ResolvedGroupUser[] = await this.findUsersForGroup(group); + + const mapped: ClassInfoDto = GroupUcMapper.mapGroupToClassInfoDto(group, resolvedUsers, system); + + return mapped; + } + private async findSystemNamesForGroups(groups: Group[]): Promise> { const systemIds: EntityId[] = groups .map((group: Group): string | undefined => group.externalSource?.systemId) diff --git a/apps/server/src/modules/group/uc/mapper/group-uc.mapper.ts b/apps/server/src/modules/group/uc/mapper/group-uc.mapper.ts index 5ac11f0e0b6..f65e8cca602 100644 --- a/apps/server/src/modules/group/uc/mapper/group-uc.mapper.ts +++ b/apps/server/src/modules/group/uc/mapper/group-uc.mapper.ts @@ -1,6 +1,6 @@ -import { RoleName, SchoolYearEntity, UserDO } from '@shared/domain'; import { Class } from '@modules/class/domain'; import { SystemDto } from '@modules/system'; +import { RoleName, SchoolYearEntity, UserDO } from '@shared/domain'; import { Group } from '../../domain'; import { ClassInfoDto, ResolvedGroupDto, ResolvedGroupUser } from '../dto'; import { ClassRootType } from '../dto/class-root-type'; @@ -16,7 +16,7 @@ export class GroupUcMapper { type: ClassRootType.GROUP, name: group.name, externalSourceName: system?.displayName, - teachers: resolvedUsers + teacherNames: resolvedUsers .filter((groupUser: ResolvedGroupUser) => groupUser.role.name === RoleName.TEACHER) .map((groupUser: ResolvedGroupUser) => groupUser.user.lastName), }); @@ -33,7 +33,7 @@ export class GroupUcMapper { type: ClassRootType.CLASS, name, externalSourceName: clazz.source, - teachers: teachers.map((user: UserDO) => user.lastName), + teacherNames: teachers.map((user: UserDO) => user.lastName), schoolYear: schoolYear?.name, isUpgradable, }); diff --git a/apps/server/src/shared/domain/interface/permission.enum.ts b/apps/server/src/shared/domain/interface/permission.enum.ts index 4512b95de7f..c3f880101b5 100644 --- a/apps/server/src/shared/domain/interface/permission.enum.ts +++ b/apps/server/src/shared/domain/interface/permission.enum.ts @@ -55,6 +55,7 @@ export enum Permission { FOLDER_CREATE = 'FOLDER_CREATE', FOLDER_DELETE = 'FOLDER_DELETE', GROUP_LIST = 'GROUP_LIST', + GROUP_FULL_ADMIN = 'GROUP_FULL_ADMIN', GROUP_VIEW = 'GROUP_VIEW', HELPDESK_CREATE = 'HELPDESK_CREATE', HELPDESK_EDIT = 'HELPDESK_EDIT', diff --git a/apps/server/src/shared/testing/user-role-permissions.ts b/apps/server/src/shared/testing/user-role-permissions.ts index cfd38cea3a3..6c38287a37e 100644 --- a/apps/server/src/shared/testing/user-role-permissions.ts +++ b/apps/server/src/shared/testing/user-role-permissions.ts @@ -139,4 +139,5 @@ export const adminPermissions = [ Permission.IMPORT_USER_UPDATE, Permission.IMPORT_USER_VIEW, Permission.SCHOOL_TOOL_ADMIN, + Permission.GROUP_FULL_ADMIN, ] as Permission[]; diff --git a/backup/setup/migrations.json b/backup/setup/migrations.json index 8292d8fc62f..90fcee0baa3 100644 --- a/backup/setup/migrations.json +++ b/backup/setup/migrations.json @@ -328,5 +328,16 @@ "$date": "2023-10-17T14:38:44.886Z" }, "__v": 0 + }, + { + "_id": { + "$oid": "653a645338f94b0ea8e3173d" + }, + "state": "up", + "name": "add-group-full-admin-permission", + "createdAt": { + "$date": "2023-10-26T13:06:27.322Z" + }, + "__v": 0 } ] diff --git a/backup/setup/roles.json b/backup/setup/roles.json index 0ad460fc526..9ceaa532f1d 100644 --- a/backup/setup/roles.json +++ b/backup/setup/roles.json @@ -134,7 +134,8 @@ "USER_LOGIN_MIGRATION_ADMIN", "START_MEETING", "JOIN_MEETING", - "GROUP_LIST" + "GROUP_LIST", + "GROUP_FULL_ADMIN" ], "__v": 2 }, @@ -191,7 +192,8 @@ "TOOL_CREATE", "TOOL_EDIT", "YEARS_EDIT", - "GROUP_LIST" + "GROUP_LIST", + "GROUP_FULL_ADMIN" ], "__v": 2 }, diff --git a/migrations/1698325587322-add-group-full-admin-permission.js b/migrations/1698325587322-add-group-full-admin-permission.js new file mode 100644 index 00000000000..cb7a10a642a --- /dev/null +++ b/migrations/1698325587322-add-group-full-admin-permission.js @@ -0,0 +1,74 @@ +const mongoose = require('mongoose'); +// eslint-disable-next-line no-unused-vars +const { info } = require('winston'); +const { alert } = require('../src/logger'); + +const { connect, close } = require('../src/utils/database'); + +const Roles = mongoose.model( + 'roles202310261524', + new mongoose.Schema( + { + name: { type: String, required: true }, + permissions: [{ type: String }], + }, + { + timestamps: true, + } + ), + 'roles' +); + +module.exports = { + up: async function up() { + // eslint-disable-next-line no-process-env + if (process.env.SC_THEME !== 'n21') { + info('Permission GROUP_FULL_ADMIN will not be added for this instance.'); + return; + } + + await connect(); + + const adminAndSuperheroRole = await Roles.updateMany( + { name: { $in: ['administrator', 'superhero'] } }, + { + $addToSet: { + permissions: { + $each: ['GROUP_FULL_ADMIN'], + }, + }, + } + ).exec(); + + if (adminAndSuperheroRole) { + alert('Permission GROUP_FULL_ADMIN added to role superhero and administrator'); + } + + await close(); + }, + + down: async function down() { + // eslint-disable-next-line no-process-env + if (process.env.SC_THEME !== 'n21') { + info('Permission GROUP_FULL_ADMIN will not be removed for this instance.'); + return; + } + + await connect(); + + const adminAndSuperheroRole = await Roles.updateMany( + { name: { $in: ['administrator', 'superhero'] } }, + { + $pull: { + permissions: 'GROUP_FULL_ADMIN', + }, + } + ).exec(); + + if (adminAndSuperheroRole) { + alert('Rollback: Removed permission GROUP_FULL_ADMIN from roles superhero and administrator'); + } + + await close(); + }, +}; From 3d78d50143b4e5c767f4eec588056b4f543be7aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= <103562092+MarvinOehlerkingCap@users.noreply.github.com> Date: Fri, 3 Nov 2023 08:16:09 +0100 Subject: [PATCH 6/8] N21-1285 User launches a CTL tool from a board card (#4511) --- .../service/column-board.service.spec.ts | 82 +++++ .../board/service/column-board.service.ts | 15 + .../auto-context-id.strategy.spec.ts | 56 ++++ .../auto-context-id.strategy.ts | 11 + .../auto-context-name.strategy.spec.ts | 207 +++++++++++++ .../auto-context-name.strategy.ts | 61 ++++ .../auto-parameter.strategy.ts | 9 + .../auto-school-id.strategy.spec.ts | 56 ++++ .../auto-school-id.strategy.ts | 15 + .../auto-school-number.strategy.spec.ts | 107 +++++++ .../auto-school-number.strategy.ts | 21 ++ .../service/auto-parameter-strategy/index.ts | 5 + .../abstract-launch.strategy.spec.ts | 287 +++++------------- .../abstract-launch.strategy.ts | 90 +++--- .../basic-tool-launch.strategy.spec.ts | 24 +- .../basic-tool-launch.strategy.ts | 0 .../{strategy => launch-strategy}/index.ts | 0 .../lti11-tool-launch.strategy.spec.ts | 30 +- .../lti11-tool-launch.strategy.ts | 20 +- .../oauth2-tool-launch.strategy.spec.ts | 26 +- .../oauth2-tool-launch.strategy.ts | 0 .../tool-launch-params.interface.ts | 2 +- .../tool-launch-strategy.interface.ts | 0 .../service/tool-launch.service.spec.ts | 2 +- .../service/tool-launch.service.ts | 2 +- .../tool/tool-launch/tool-launch.module.ts | 18 +- backup/setup/external_tools.json | 4 +- 27 files changed, 854 insertions(+), 296 deletions(-) create mode 100644 apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.spec.ts create mode 100644 apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.ts create mode 100644 apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.spec.ts create mode 100644 apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.ts create mode 100644 apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-parameter.strategy.ts create mode 100644 apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.spec.ts create mode 100644 apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.ts create mode 100644 apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.spec.ts create mode 100644 apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.ts create mode 100644 apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/index.ts rename apps/server/src/modules/tool/tool-launch/service/{strategy => launch-strategy}/abstract-launch.strategy.spec.ts (67%) rename apps/server/src/modules/tool/tool-launch/service/{strategy => launch-strategy}/abstract-launch.strategy.ts (81%) rename apps/server/src/modules/tool/tool-launch/service/{strategy => launch-strategy}/basic-tool-launch.strategy.spec.ts (90%) rename apps/server/src/modules/tool/tool-launch/service/{strategy => launch-strategy}/basic-tool-launch.strategy.ts (100%) rename apps/server/src/modules/tool/tool-launch/service/{strategy => launch-strategy}/index.ts (100%) rename apps/server/src/modules/tool/tool-launch/service/{strategy => launch-strategy}/lti11-tool-launch.strategy.spec.ts (96%) rename apps/server/src/modules/tool/tool-launch/service/{strategy => launch-strategy}/lti11-tool-launch.strategy.ts (92%) rename apps/server/src/modules/tool/tool-launch/service/{strategy => launch-strategy}/oauth2-tool-launch.strategy.spec.ts (80%) rename apps/server/src/modules/tool/tool-launch/service/{strategy => launch-strategy}/oauth2-tool-launch.strategy.ts (100%) rename apps/server/src/modules/tool/tool-launch/service/{strategy => launch-strategy}/tool-launch-params.interface.ts (100%) rename apps/server/src/modules/tool/tool-launch/service/{strategy => launch-strategy}/tool-launch-strategy.interface.ts (100%) diff --git a/apps/server/src/modules/board/service/column-board.service.spec.ts b/apps/server/src/modules/board/service/column-board.service.spec.ts index 6ed6bb6f0a4..97b4a3d2578 100644 --- a/apps/server/src/modules/board/service/column-board.service.spec.ts +++ b/apps/server/src/modules/board/service/column-board.service.spec.ts @@ -2,6 +2,7 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { IConfig } from '@hpi-schul-cloud/commons/lib/interfaces/IConfig'; import { Test, TestingModule } from '@nestjs/testing'; +import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { BoardExternalReference, BoardExternalReferenceType, @@ -106,6 +107,87 @@ describe(ColumnBoardService.name, () => { }); }); + describe('findByDescendant', () => { + describe('when searching a board for an element', () => { + const setup2 = () => { + const element = richTextElementFactory.build(); + const board: ColumnBoard = columnBoardFactory.build({ children: [element] }); + + boardDoRepo.getAncestorIds.mockResolvedValue([board.id]); + boardDoRepo.findById.mockResolvedValue(board); + + return { + element, + board, + }; + }; + + it('should search by the root id', async () => { + const { element, board } = setup2(); + + await service.findByDescendant(element); + + expect(boardDoRepo.findById).toHaveBeenCalledWith(board.id, 1); + }); + + it('should return the board', async () => { + const { element, board } = setup2(); + + const result = await service.findByDescendant(element); + + expect(result).toEqual(board); + }); + }); + + describe('when searching a board by itself', () => { + const setup2 = () => { + const board: ColumnBoard = columnBoardFactory.build({ children: [] }); + + boardDoRepo.getAncestorIds.mockResolvedValue([]); + boardDoRepo.findById.mockResolvedValue(board); + + return { + board, + }; + }; + + it('should search by the root id', async () => { + const { board } = setup2(); + + await service.findByDescendant(board); + + expect(boardDoRepo.findById).toHaveBeenCalledWith(board.id, 1); + }); + + it('should return the board', async () => { + const { board } = setup2(); + + const result = await service.findByDescendant(board); + + expect(result).toEqual(board); + }); + }); + + describe('when the root node is not a board', () => { + const setup2 = () => { + const element = richTextElementFactory.build(); + + boardDoRepo.getAncestorIds.mockResolvedValue([]); + boardDoRepo.findById.mockResolvedValue(element); + + return { + element, + }; + }; + + it('should throw a NotFoundLoggableException', async () => { + const { element } = setup2(); + + await expect(service.findByDescendant(element)).rejects.toThrow(NotFoundLoggableException); + }); + }); + }); + describe('getBoardObjectTitlesById', () => { describe('when asking for a list of boardObject-ids', () => { const setupBoards = () => { diff --git a/apps/server/src/modules/board/service/column-board.service.ts b/apps/server/src/modules/board/service/column-board.service.ts index f53f4f5f051..7a455e0fcfa 100644 --- a/apps/server/src/modules/board/service/column-board.service.ts +++ b/apps/server/src/modules/board/service/column-board.service.ts @@ -1,6 +1,8 @@ import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { Injectable } from '@nestjs/common'; +import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { + AnyBoardDo, BoardExternalReference, Card, Column, @@ -34,6 +36,19 @@ export class ColumnBoardService { return ids; } + async findByDescendant(boardDo: AnyBoardDo): Promise { + const ancestorIds: EntityId[] = await this.boardDoRepo.getAncestorIds(boardDo); + const idHierarchy: EntityId[] = [...ancestorIds, boardDo.id]; + const rootId: EntityId = idHierarchy[0]; + const rootBoardDo: AnyBoardDo = await this.boardDoRepo.findById(rootId, 1); + + if (rootBoardDo instanceof ColumnBoard) { + return rootBoardDo; + } + + throw new NotFoundLoggableException(ColumnBoard.name, 'id', rootId); + } + async getBoardObjectTitlesById(boardIds: EntityId[]): Promise> { const titleMap = this.boardDoRepo.getTitlesByIds(boardIds); return titleMap; diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.spec.ts new file mode 100644 index 00000000000..d1865d231c2 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.spec.ts @@ -0,0 +1,56 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { contextExternalToolFactory, schoolExternalToolFactory } from '@shared/testing'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoContextIdStrategy } from './auto-context-id.strategy'; + +describe(AutoContextIdStrategy.name, () => { + let module: TestingModule; + let strategy: AutoContextIdStrategy; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [AutoContextIdStrategy], + }).compile(); + + strategy = module.get(AutoContextIdStrategy); + }); + + afterAll(async () => { + await module.close(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('getValue', () => { + const setup = () => { + const contextId = 'contextId'; + + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId({ + schoolToolRef: { + schoolToolId: schoolExternalTool.id as string, + }, + contextRef: { + id: contextId, + }, + }); + + return { + contextId, + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return the context id', () => { + const { contextId, schoolExternalTool, contextExternalTool } = setup(); + + const result: string | undefined = strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toEqual(contextId); + }); + }); +}); diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.ts new file mode 100644 index 00000000000..a732d038522 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-id.strategy.ts @@ -0,0 +1,11 @@ +import { Injectable } from '@nestjs/common'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoParameterStrategy } from './auto-parameter.strategy'; + +@Injectable() +export class AutoContextIdStrategy implements AutoParameterStrategy { + getValue(schoolExternalTool: SchoolExternalTool, contextExternalTool: ContextExternalTool): string | undefined { + return contextExternalTool.contextRef.id; + } +} diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.spec.ts new file mode 100644 index 00000000000..caa02d1c69b --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.spec.ts @@ -0,0 +1,207 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { ObjectId } from '@mikro-orm/mongodb'; +import { ColumnBoardService, ContentElementService } from '@modules/board'; +import { CourseService } from '@modules/learnroom'; +import { Test, TestingModule } from '@nestjs/testing'; +import { BoardExternalReferenceType, ColumnBoard, Course, ExternalToolElement } from '@shared/domain'; +import { + columnBoardFactory, + contextExternalToolFactory, + courseFactory, + externalToolElementFactory, + schoolExternalToolFactory, + setupEntities, +} from '@shared/testing'; +import { ToolContextType } from '../../../common/enum'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { ParameterTypeNotImplementedLoggableException } from '../../error'; +import { AutoContextNameStrategy } from './auto-context-name.strategy'; + +describe(AutoContextNameStrategy.name, () => { + let module: TestingModule; + let strategy: AutoContextNameStrategy; + + let courseService: DeepMocked; + let contentElementService: DeepMocked; + let columnBoardService: DeepMocked; + + beforeAll(async () => { + await setupEntities(); + + module = await Test.createTestingModule({ + providers: [ + AutoContextNameStrategy, + { + provide: CourseService, + useValue: createMock(), + }, + { + provide: ContentElementService, + useValue: createMock(), + }, + { + provide: ColumnBoardService, + useValue: createMock(), + }, + ], + }).compile(); + + strategy = module.get(AutoContextNameStrategy); + courseService = module.get(CourseService); + contentElementService = module.get(ContentElementService); + columnBoardService = module.get(ColumnBoardService); + }); + + afterAll(async () => { + await module.close(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('getValue', () => { + describe('when the tool context is "course"', () => { + const setup = () => { + const courseId = new ObjectId().toHexString(); + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ + contextRef: { + id: courseId, + type: ToolContextType.COURSE, + }, + }); + + const course: Course = courseFactory.buildWithId( + { + name: 'testName', + }, + courseId + ); + + courseService.findById.mockResolvedValue(course); + + return { + schoolExternalTool, + contextExternalTool, + course, + }; + }; + + it('should return the course name', async () => { + const { schoolExternalTool, contextExternalTool, course } = setup(); + + const result: string | undefined = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toEqual(course.name); + }); + }); + + describe('when the tool context is "board element" and the board context is "course"', () => { + const setup = () => { + const boardElementId = new ObjectId().toHexString(); + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ + contextRef: { + id: boardElementId, + type: ToolContextType.BOARD_ELEMENT, + }, + }); + + const course: Course = courseFactory.buildWithId({ + name: 'testName', + }); + + const externalToolElement: ExternalToolElement = externalToolElementFactory.build(); + + const columnBoard: ColumnBoard = columnBoardFactory.build({ + context: { + id: course.id, + type: BoardExternalReferenceType.Course, + }, + }); + + courseService.findById.mockResolvedValue(course); + contentElementService.findById.mockResolvedValue(externalToolElement); + columnBoardService.findByDescendant.mockResolvedValue(columnBoard); + + return { + schoolExternalTool, + contextExternalTool, + course, + }; + }; + + it('should return the course name', async () => { + const { schoolExternalTool, contextExternalTool, course } = setup(); + + const result: string | undefined = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toEqual(course.name); + }); + }); + + describe('when the tool context is "board element" and the board context is unknown', () => { + const setup = () => { + const boardElementId = new ObjectId().toHexString(); + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ + contextRef: { + id: boardElementId, + type: ToolContextType.BOARD_ELEMENT, + }, + }); + + const externalToolElement: ExternalToolElement = externalToolElementFactory.build(); + + const columnBoard: ColumnBoard = columnBoardFactory.build({ + context: { + id: new ObjectId().toHexString(), + type: 'unknown' as unknown as BoardExternalReferenceType, + }, + }); + + contentElementService.findById.mockResolvedValue(externalToolElement); + columnBoardService.findByDescendant.mockResolvedValue(columnBoard); + + return { + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return undefined', async () => { + const { schoolExternalTool, contextExternalTool } = setup(); + + const result: string | undefined = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toBeUndefined(); + }); + }); + + describe('when a lookup for a context name is not implemented', () => { + const setup = () => { + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ + contextRef: { + type: 'unknownContext' as unknown as ToolContextType, + }, + }); + + return { + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should throw a ParameterNotImplementedLoggableException', async () => { + const { schoolExternalTool, contextExternalTool } = setup(); + + await expect(strategy.getValue(schoolExternalTool, contextExternalTool)).rejects.toThrow( + ParameterTypeNotImplementedLoggableException + ); + }); + }); + }); +}); diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.ts new file mode 100644 index 00000000000..14d296d8b60 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-context-name.strategy.ts @@ -0,0 +1,61 @@ +import { ColumnBoardService, ContentElementService } from '@modules/board'; +import { CourseService } from '@modules/learnroom'; +import { Injectable } from '@nestjs/common'; +import { AnyContentElementDo, BoardExternalReferenceType, ColumnBoard, Course, EntityId } from '@shared/domain'; +import { CustomParameterType, ToolContextType } from '../../../common/enum'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { ParameterTypeNotImplementedLoggableException } from '../../error'; +import { AutoParameterStrategy } from './auto-parameter.strategy'; + +@Injectable() +export class AutoContextNameStrategy implements AutoParameterStrategy { + constructor( + private readonly courseService: CourseService, + private readonly contentElementService: ContentElementService, + private readonly columnBoardService: ColumnBoardService + ) {} + + async getValue( + schoolExternalTool: SchoolExternalTool, + contextExternalTool: ContextExternalTool + ): Promise { + switch (contextExternalTool.contextRef.type) { + case ToolContextType.COURSE: { + const courseValue: string = await this.getCourseValue(contextExternalTool.contextRef.id); + + return courseValue; + } + case ToolContextType.BOARD_ELEMENT: { + const boardValue: string | undefined = await this.getBoardValue(contextExternalTool.contextRef.id); + + return boardValue; + } + default: { + throw new ParameterTypeNotImplementedLoggableException( + `${CustomParameterType.AUTO_CONTEXTNAME}/${contextExternalTool.contextRef.type as string}` + ); + } + } + } + + private async getCourseValue(courseId: EntityId): Promise { + const course: Course = await this.courseService.findById(courseId); + + return course.name; + } + + private async getBoardValue(elementId: EntityId): Promise { + const element: AnyContentElementDo = await this.contentElementService.findById(elementId); + + const board: ColumnBoard = await this.columnBoardService.findByDescendant(element); + + if (board.context.type === BoardExternalReferenceType.Course) { + const courseName: string = await this.getCourseValue(board.context.id); + + return courseName; + } + + return undefined; + } +} diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-parameter.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-parameter.strategy.ts new file mode 100644 index 00000000000..5c5efbcc2b1 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-parameter.strategy.ts @@ -0,0 +1,9 @@ +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; + +export interface AutoParameterStrategy { + getValue( + schoolExternalTool: SchoolExternalTool, + contextExternalTool: ContextExternalTool + ): string | Promise | undefined; +} diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.spec.ts new file mode 100644 index 00000000000..2b184c51140 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.spec.ts @@ -0,0 +1,56 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { contextExternalToolFactory, schoolExternalToolFactory } from '@shared/testing'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoSchoolIdStrategy } from './auto-school-id.strategy'; + +describe(AutoSchoolIdStrategy.name, () => { + let module: TestingModule; + let strategy: AutoSchoolIdStrategy; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [AutoSchoolIdStrategy], + }).compile(); + + strategy = module.get(AutoSchoolIdStrategy); + }); + + afterAll(async () => { + await module.close(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('getValue', () => { + const setup = () => { + const schoolId = 'schoolId'; + + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId({ + schoolId, + }); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId({ + schoolToolRef: { + schoolToolId: schoolExternalTool.id as string, + schoolId, + }, + }); + + return { + schoolId, + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return the context id', () => { + const { schoolId, schoolExternalTool, contextExternalTool } = setup(); + + const result: string | undefined = strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toEqual(schoolId); + }); + }); +}); diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.ts new file mode 100644 index 00000000000..faad4be07d9 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-id.strategy.ts @@ -0,0 +1,15 @@ +import { Injectable } from '@nestjs/common'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoParameterStrategy } from './auto-parameter.strategy'; + +@Injectable() +export class AutoSchoolIdStrategy implements AutoParameterStrategy { + getValue( + schoolExternalTool: SchoolExternalTool, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + contextExternalTool: ContextExternalTool + ): string | undefined { + return schoolExternalTool.schoolId; + } +} diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.spec.ts new file mode 100644 index 00000000000..91a01bbdd39 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.spec.ts @@ -0,0 +1,107 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { LegacySchoolService } from '@modules/legacy-school'; +import { Test, TestingModule } from '@nestjs/testing'; +import { LegacySchoolDo } from '@shared/domain'; +import { contextExternalToolFactory, legacySchoolDoFactory, schoolExternalToolFactory } from '@shared/testing'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoSchoolNumberStrategy } from './auto-school-number.strategy'; + +describe(AutoSchoolNumberStrategy.name, () => { + let module: TestingModule; + let strategy: AutoSchoolNumberStrategy; + + let schoolService: DeepMocked; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [ + AutoSchoolNumberStrategy, + { + provide: LegacySchoolService, + useValue: createMock(), + }, + ], + }).compile(); + + strategy = module.get(AutoSchoolNumberStrategy); + schoolService = module.get(LegacySchoolService); + }); + + afterAll(async () => { + await module.close(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('getValue', () => { + describe('when the school has a school number', () => { + const setup = () => { + const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId({ + officialSchoolNumber: 'officialSchoolNumber', + }); + + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId({ + schoolId: school.id as string, + }); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId({ + schoolToolRef: { + schoolToolId: schoolExternalTool.id as string, + schoolId: school.id, + }, + }); + + schoolService.getSchoolById.mockResolvedValue(school); + + return { + school, + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return the school number', async () => { + const { school, schoolExternalTool, contextExternalTool } = setup(); + + const result: string | undefined = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toEqual(school.officialSchoolNumber); + }); + }); + + describe('when the school does not have a school number', () => { + const setup = () => { + const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId({ + officialSchoolNumber: undefined, + }); + + const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId({ + schoolId: school.id as string, + }); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId({ + schoolToolRef: { + schoolToolId: schoolExternalTool.id as string, + schoolId: school.id, + }, + }); + + schoolService.getSchoolById.mockResolvedValue(school); + + return { + schoolExternalTool, + contextExternalTool, + }; + }; + + it('should return undefined', async () => { + const { schoolExternalTool, contextExternalTool } = setup(); + + const result: string | undefined = await strategy.getValue(schoolExternalTool, contextExternalTool); + + expect(result).toBeUndefined(); + }); + }); + }); +}); diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.ts new file mode 100644 index 00000000000..7d9ad9dad65 --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/auto-school-number.strategy.ts @@ -0,0 +1,21 @@ +import { LegacySchoolService } from '@modules/legacy-school'; +import { Injectable } from '@nestjs/common'; +import { LegacySchoolDo } from '@shared/domain'; +import { ContextExternalTool } from '../../../context-external-tool/domain'; +import { SchoolExternalTool } from '../../../school-external-tool/domain'; +import { AutoParameterStrategy } from './auto-parameter.strategy'; + +@Injectable() +export class AutoSchoolNumberStrategy implements AutoParameterStrategy { + constructor(private readonly schoolService: LegacySchoolService) {} + + async getValue( + schoolExternalTool: SchoolExternalTool, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + contextExternalTool: ContextExternalTool + ): Promise { + const school: LegacySchoolDo = await this.schoolService.getSchoolById(schoolExternalTool.schoolId); + + return school.officialSchoolNumber; + } +} diff --git a/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/index.ts b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/index.ts new file mode 100644 index 00000000000..619a0a6296c --- /dev/null +++ b/apps/server/src/modules/tool/tool-launch/service/auto-parameter-strategy/index.ts @@ -0,0 +1,5 @@ +export * from './auto-parameter.strategy'; +export * from './auto-school-id.strategy'; +export * from './auto-context-id.strategy'; +export * from './auto-context-name.strategy'; +export * from './auto-school-number.strategy'; diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/abstract-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts similarity index 67% rename from apps/server/src/modules/tool/tool-launch/service/strategy/abstract-launch.strategy.spec.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts index 7ee237b3e93..04868663671 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/abstract-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.spec.ts @@ -2,25 +2,15 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; import { Injectable } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; -import { Course, EntityId, LegacySchoolDo } from '@shared/domain'; +import { EntityId } from '@shared/domain'; import { contextExternalToolFactory, - courseFactory, customParameterFactory, externalToolFactory, - legacySchoolDoFactory, schoolExternalToolFactory, - setupEntities, } from '@shared/testing'; -import { CourseService } from '@modules/learnroom/service'; -import { LegacySchoolService } from '@modules/legacy-school'; import { CustomParameterEntry } from '../../../common/domain'; -import { - CustomParameterLocation, - CustomParameterScope, - CustomParameterType, - ToolContextType, -} from '../../../common/enum'; +import { CustomParameterLocation, CustomParameterScope, CustomParameterType } from '../../../common/enum'; import { ContextExternalTool } from '../../../context-external-tool/domain'; import { ExternalTool } from '../../../external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; @@ -33,6 +23,12 @@ import { ToolLaunchDataType, ToolLaunchRequest, } from '../../types'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from '../auto-parameter-strategy'; import { AbstractLaunchStrategy } from './abstract-launch.strategy'; import { IToolLaunchParams } from './tool-launch-params.interface'; @@ -69,33 +65,44 @@ class TestLaunchStrategy extends AbstractLaunchStrategy { } } -describe('AbstractLaunchStrategy', () => { +describe(AbstractLaunchStrategy.name, () => { let module: TestingModule; - let launchStrategy: TestLaunchStrategy; + let strategy: TestLaunchStrategy; - let schoolService: DeepMocked; - let courseService: DeepMocked; + let autoSchoolIdStrategy: DeepMocked; + let autoSchoolNumberStrategy: DeepMocked; + let autoContextIdStrategy: DeepMocked; + let autoContextNameStrategy: DeepMocked; beforeAll(async () => { - await setupEntities(); - module = await Test.createTestingModule({ providers: [ TestLaunchStrategy, { - provide: LegacySchoolService, - useValue: createMock(), + provide: AutoSchoolIdStrategy, + useValue: createMock(), + }, + { + provide: AutoSchoolNumberStrategy, + useValue: createMock(), }, { - provide: CourseService, - useValue: createMock(), + provide: AutoContextIdStrategy, + useValue: createMock(), + }, + { + provide: AutoContextNameStrategy, + useValue: createMock(), }, ], }).compile(); - launchStrategy = module.get(TestLaunchStrategy); - schoolService = module.get(LegacySchoolService); - courseService = module.get(CourseService); + strategy = module.get(TestLaunchStrategy); + + autoSchoolIdStrategy = module.get(AutoSchoolIdStrategy); + autoSchoolNumberStrategy = module.get(AutoSchoolNumberStrategy); + autoContextIdStrategy = module.get(AutoContextIdStrategy); + autoContextNameStrategy = module.get(AutoContextNameStrategy); }); afterAll(async () => { @@ -106,6 +113,7 @@ describe('AbstractLaunchStrategy', () => { describe('when parameters of every type are defined', () => { const setup = () => { const schoolId: string = new ObjectId().toHexString(); + const mockedAutoValue = 'mockedAutoValue'; // External Tool const globalCustomParameter = customParameterFactory.build({ @@ -139,6 +147,18 @@ describe('AbstractLaunchStrategy', () => { name: 'autoSchoolNumberParam', type: CustomParameterType.AUTO_SCHOOLNUMBER, }); + const autoContextIdCustomParameter = customParameterFactory.build({ + scope: CustomParameterScope.GLOBAL, + location: CustomParameterLocation.BODY, + name: 'autoSchoolNumberParam', + type: CustomParameterType.AUTO_CONTEXTID, + }); + const autoContextNameCustomParameter = customParameterFactory.build({ + scope: CustomParameterScope.GLOBAL, + location: CustomParameterLocation.BODY, + name: 'autoSchoolNumberParam', + type: CustomParameterType.AUTO_CONTEXTNAME, + }); const externalTool: ExternalTool = externalToolFactory.build({ parameters: [ @@ -147,6 +167,8 @@ describe('AbstractLaunchStrategy', () => { contextCustomParameter, autoSchoolIdCustomParameter, autoSchoolNumberCustomParameter, + autoContextIdCustomParameter, + autoContextNameCustomParameter, ], }); @@ -169,16 +191,6 @@ describe('AbstractLaunchStrategy', () => { parameters: [contextParameterEntry], }); - // Other - const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId( - { - officialSchoolNumber: '1234', - }, - schoolId - ); - - schoolService.getSchoolById.mockResolvedValue(school); - const sortFn = (a: PropertyData, b: PropertyData) => { if (a.name < b.name) { return -1; @@ -189,17 +201,24 @@ describe('AbstractLaunchStrategy', () => { return 0; }; + autoSchoolIdStrategy.getValue.mockReturnValueOnce(mockedAutoValue); + autoSchoolNumberStrategy.getValue.mockResolvedValueOnce(mockedAutoValue); + autoContextIdStrategy.getValue.mockReturnValueOnce(mockedAutoValue); + autoContextNameStrategy.getValue.mockResolvedValueOnce(mockedAutoValue); + return { globalCustomParameter, schoolCustomParameter, autoSchoolIdCustomParameter, autoSchoolNumberCustomParameter, + autoContextIdCustomParameter, + autoContextNameCustomParameter, schoolParameterEntry, contextParameterEntry, externalTool, schoolExternalTool, contextExternalTool, - school, + mockedAutoValue, sortFn, }; }; @@ -211,15 +230,17 @@ describe('AbstractLaunchStrategy', () => { contextParameterEntry, autoSchoolIdCustomParameter, autoSchoolNumberCustomParameter, + autoContextIdCustomParameter, + autoContextNameCustomParameter, schoolParameterEntry, externalTool, schoolExternalTool, contextExternalTool, - school, + mockedAutoValue, sortFn, } = setup(); - const result: ToolLaunchData = await launchStrategy.createLaunchData('userId', { + const result: ToolLaunchData = await strategy.createLaunchData('userId', { externalTool, schoolExternalTool, contextExternalTool, @@ -248,135 +269,22 @@ describe('AbstractLaunchStrategy', () => { }, { name: autoSchoolIdCustomParameter.name, - value: school.id as string, + value: mockedAutoValue, location: PropertyLocation.BODY, }, { name: autoSchoolNumberCustomParameter.name, - value: school.officialSchoolNumber as string, + value: mockedAutoValue, location: PropertyLocation.BODY, }, { - name: concreteConfigParameter.name, - value: concreteConfigParameter.value, - location: concreteConfigParameter.location, - }, - ].sort(sortFn), - }); - }); - }); - - describe('when launching with context name parameter for the context "course"', () => { - const setup = () => { - const autoCourseNameCustomParameter = customParameterFactory.build({ - scope: CustomParameterScope.GLOBAL, - location: CustomParameterLocation.BODY, - name: 'autoCourseNameParam', - type: CustomParameterType.AUTO_CONTEXTNAME, - }); - - const externalTool: ExternalTool = externalToolFactory.build({ - parameters: [autoCourseNameCustomParameter], - }); - - const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); - - const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ - contextRef: { - type: ToolContextType.COURSE, - }, - }); - - const course: Course = courseFactory.buildWithId( - { - name: 'testName', - }, - contextExternalTool.contextRef.id - ); - - courseService.findById.mockResolvedValue(course); - - return { - autoCourseNameCustomParameter, - externalTool, - schoolExternalTool, - contextExternalTool, - course, - }; - }; - - it('should return ToolLaunchData with the course name as parameter value', async () => { - const { externalTool, schoolExternalTool, contextExternalTool, autoCourseNameCustomParameter, course } = - setup(); - - const result: ToolLaunchData = await launchStrategy.createLaunchData('userId', { - externalTool, - schoolExternalTool, - contextExternalTool, - }); - - expect(result).toEqual({ - baseUrl: externalTool.config.baseUrl, - type: ToolLaunchDataType.BASIC, - openNewTab: false, - properties: [ - { - name: autoCourseNameCustomParameter.name, - value: course.name, + name: autoContextIdCustomParameter.name, + value: mockedAutoValue, location: PropertyLocation.BODY, }, { - name: concreteConfigParameter.name, - value: concreteConfigParameter.value, - location: concreteConfigParameter.location, - }, - ], - }); - }); - }); - - describe('when launching with context id parameter', () => { - const setup = () => { - const autoContextIdCustomParameter = customParameterFactory.build({ - scope: CustomParameterScope.GLOBAL, - location: CustomParameterLocation.BODY, - name: 'autoContextIdParam', - type: CustomParameterType.AUTO_CONTEXTID, - }); - - const externalTool: ExternalTool = externalToolFactory.build({ - parameters: [autoContextIdCustomParameter], - }); - - const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build(); - - const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build(); - - return { - autoContextIdCustomParameter, - externalTool, - schoolExternalTool, - contextExternalTool, - }; - }; - - it('should return ToolLaunchData with the context id as parameter value', async () => { - const { externalTool, schoolExternalTool, contextExternalTool, autoContextIdCustomParameter } = setup(); - - const result: ToolLaunchData = await launchStrategy.createLaunchData('userId', { - externalTool, - schoolExternalTool, - contextExternalTool, - }); - - expect(result).toEqual({ - baseUrl: externalTool.config.baseUrl, - type: ToolLaunchDataType.BASIC, - openNewTab: false, - properties: [ - { - name: autoContextIdCustomParameter.name, - value: contextExternalTool.contextRef.id, + name: autoContextNameCustomParameter.name, + value: mockedAutoValue, location: PropertyLocation.BODY, }, { @@ -384,7 +292,7 @@ describe('AbstractLaunchStrategy', () => { value: concreteConfigParameter.value, location: concreteConfigParameter.location, }, - ], + ].sort(sortFn), }); }); }); @@ -413,7 +321,7 @@ describe('AbstractLaunchStrategy', () => { it('should return a ToolLaunchData with no custom parameters', async () => { const { externalTool, schoolExternalTool, contextExternalTool } = setup(); - const result: ToolLaunchData = await launchStrategy.createLaunchData('userId', { + const result: ToolLaunchData = await strategy.createLaunchData('userId', { externalTool, schoolExternalTool, contextExternalTool, @@ -454,11 +362,7 @@ describe('AbstractLaunchStrategy', () => { parameters: [], }); - const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId({ - officialSchoolNumber: undefined, - }); - - schoolService.getSchoolById.mockResolvedValue(school); + autoSchoolNumberStrategy.getValue.mockResolvedValue(undefined); return { externalTool, @@ -471,7 +375,7 @@ describe('AbstractLaunchStrategy', () => { const { externalTool, schoolExternalTool, contextExternalTool } = setup(); const func = async () => - launchStrategy.createLaunchData('userId', { + strategy.createLaunchData('userId', { externalTool, schoolExternalTool, contextExternalTool, @@ -512,52 +416,7 @@ describe('AbstractLaunchStrategy', () => { const { externalTool, schoolExternalTool, contextExternalTool } = setup(); const func = async () => - launchStrategy.createLaunchData('userId', { - externalTool, - schoolExternalTool, - contextExternalTool, - }); - - await expect(func).rejects.toThrow(ParameterTypeNotImplementedLoggableException); - }); - }); - - describe('when a lookup for a context name is not implemented', () => { - const setup = () => { - const customParameterWithUnknownType = customParameterFactory.build({ - scope: CustomParameterScope.GLOBAL, - location: CustomParameterLocation.BODY, - name: 'autoContextNameParam', - type: CustomParameterType.AUTO_CONTEXTNAME, - }); - const externalTool: ExternalTool = externalToolFactory.build({ - parameters: [customParameterWithUnknownType], - }); - - const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.build({ - parameters: [], - }); - - const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build({ - contextRef: { - id: new ObjectId().toHexString(), - type: 'unknownContext' as unknown as ToolContextType, - }, - parameters: [], - }); - - return { - externalTool, - schoolExternalTool, - contextExternalTool, - }; - }; - - it('should throw a ParameterNotImplementedLoggableException', async () => { - const { externalTool, schoolExternalTool, contextExternalTool } = setup(); - - const func = async () => - launchStrategy.createLaunchData('userId', { + strategy.createLaunchData('userId', { externalTool, schoolExternalTool, contextExternalTool, @@ -597,7 +456,7 @@ describe('AbstractLaunchStrategy', () => { }); toolLaunchDataDO.properties = [propertyData1, propertyData2]; - const result: ToolLaunchRequest = launchStrategy.createLaunchRequest(toolLaunchDataDO); + const result: ToolLaunchRequest = strategy.createLaunchRequest(toolLaunchDataDO); expect(result).toEqual({ method: LaunchRequestMethod.GET, @@ -622,7 +481,7 @@ describe('AbstractLaunchStrategy', () => { }); toolLaunchDataDO.properties = [bodyProperty1, bodyProperty2]; - const result: ToolLaunchRequest = launchStrategy.createLaunchRequest(toolLaunchDataDO); + const result: ToolLaunchRequest = strategy.createLaunchRequest(toolLaunchDataDO); expect(result.payload).toEqual(expectedPayload); }); @@ -642,7 +501,7 @@ describe('AbstractLaunchStrategy', () => { }); toolLaunchDataDO.properties = [pathProperty, queryProperty]; - const result: ToolLaunchRequest = launchStrategy.createLaunchRequest(toolLaunchDataDO); + const result: ToolLaunchRequest = strategy.createLaunchRequest(toolLaunchDataDO); expect(result.url).toEqual( `https://www.basic-baseurl.com/pre/${pathProperty.value}/post?${queryProperty.name}=${queryProperty.value}` diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/abstract-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts similarity index 81% rename from apps/server/src/modules/tool/tool-launch/service/strategy/abstract-launch.strategy.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts index 63ba0680734..ce0f07c5731 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/abstract-launch.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/abstract-launch.strategy.ts @@ -1,27 +1,41 @@ import { Injectable } from '@nestjs/common'; -import { Course, EntityId, LegacySchoolDo } from '@shared/domain'; -import { CourseService } from '@modules/learnroom/service'; -import { LegacySchoolService } from '@modules/legacy-school'; +import { EntityId } from '@shared/domain'; import { URLSearchParams } from 'url'; import { CustomParameter, CustomParameterEntry } from '../../../common/domain'; -import { - CustomParameterLocation, - CustomParameterScope, - CustomParameterType, - ToolContextType, -} from '../../../common/enum'; +import { CustomParameterLocation, CustomParameterScope, CustomParameterType } from '../../../common/enum'; import { ContextExternalTool } from '../../../context-external-tool/domain'; import { ExternalTool } from '../../../external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { MissingToolParameterValueLoggableException, ParameterTypeNotImplementedLoggableException } from '../../error'; import { ToolLaunchMapper } from '../../mapper'; import { LaunchRequestMethod, PropertyData, PropertyLocation, ToolLaunchData, ToolLaunchRequest } from '../../types'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoParameterStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from '../auto-parameter-strategy'; import { IToolLaunchParams } from './tool-launch-params.interface'; import { IToolLaunchStrategy } from './tool-launch-strategy.interface'; @Injectable() export abstract class AbstractLaunchStrategy implements IToolLaunchStrategy { - constructor(private readonly schoolService: LegacySchoolService, private readonly courseService: CourseService) {} + private readonly autoParameterStrategyMap: Map; + + constructor( + autoSchoolIdStrategy: AutoSchoolIdStrategy, + autoSchoolNumberStrategy: AutoSchoolNumberStrategy, + autoContextIdStrategy: AutoContextIdStrategy, + autoContextNameStrategy: AutoContextNameStrategy + ) { + this.autoParameterStrategyMap = new Map([ + [CustomParameterType.AUTO_SCHOOLID, autoSchoolIdStrategy], + [CustomParameterType.AUTO_SCHOOLNUMBER, autoSchoolNumberStrategy], + [CustomParameterType.AUTO_CONTEXTID, autoContextIdStrategy], + [CustomParameterType.AUTO_CONTEXTNAME, autoContextNameStrategy], + ]); + } public async createLaunchData(userId: EntityId, data: IToolLaunchParams): Promise { const launchData: ToolLaunchData = this.buildToolLaunchDataFromExternalTool(data.externalTool); @@ -207,43 +221,29 @@ export abstract class AbstractLaunchStrategy implements IToolLaunchStrategy { schoolExternalTool: SchoolExternalTool, contextExternalTool: ContextExternalTool ): Promise { - switch (customParameter.type) { - case CustomParameterType.AUTO_SCHOOLID: { - return schoolExternalTool.schoolId; - } - case CustomParameterType.AUTO_CONTEXTID: { - return contextExternalTool.contextRef.id; - } - case CustomParameterType.AUTO_CONTEXTNAME: { - switch (contextExternalTool.contextRef.type) { - case ToolContextType.COURSE: { - const course: Course = await this.courseService.findById(contextExternalTool.contextRef.id); - - return course.name; - } - default: { - throw new ParameterTypeNotImplementedLoggableException( - `${customParameter.type}/${contextExternalTool.contextRef.type as string}` - ); - } - } - } - case CustomParameterType.AUTO_SCHOOLNUMBER: { - const school: LegacySchoolDo = await this.schoolService.getSchoolById(schoolExternalTool.schoolId); + if ( + customParameter.type === CustomParameterType.BOOLEAN || + customParameter.type === CustomParameterType.NUMBER || + customParameter.type === CustomParameterType.STRING + ) { + return customParameter.scope === CustomParameterScope.GLOBAL + ? customParameter.default + : matchingParameterEntry?.value; + } - return school.officialSchoolNumber; - } - case CustomParameterType.BOOLEAN: - case CustomParameterType.NUMBER: - case CustomParameterType.STRING: { - return customParameter.scope === CustomParameterScope.GLOBAL - ? customParameter.default - : matchingParameterEntry?.value; - } - default: { - throw new ParameterTypeNotImplementedLoggableException(customParameter.type); - } + const autoParameterStrategy: AutoParameterStrategy | undefined = this.autoParameterStrategyMap.get( + customParameter.type + ); + if (autoParameterStrategy) { + const autoValue: string | undefined = await autoParameterStrategy.getValue( + schoolExternalTool, + contextExternalTool + ); + + return autoValue; } + + throw new ParameterTypeNotImplementedLoggableException(customParameter.type); } private addProperty( diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/basic-tool-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts similarity index 90% rename from apps/server/src/modules/tool/tool-launch/service/strategy/basic-tool-launch.strategy.spec.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts index 3bb95b97755..db80f78498f 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/basic-tool-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.spec.ts @@ -1,12 +1,16 @@ import { createMock } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; import { contextExternalToolFactory, externalToolFactory, schoolExternalToolFactory } from '@shared/testing'; -import { CourseService } from '@modules/learnroom/service'; -import { LegacySchoolService } from '@modules/legacy-school'; import { ContextExternalTool } from '../../../context-external-tool/domain'; import { ExternalTool } from '../../../external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { LaunchRequestMethod, PropertyData, PropertyLocation } from '../../types'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from '../auto-parameter-strategy'; import { BasicToolLaunchStrategy } from './basic-tool-launch.strategy'; import { IToolLaunchParams } from './tool-launch-params.interface'; @@ -19,12 +23,20 @@ describe('BasicToolLaunchStrategy', () => { providers: [ BasicToolLaunchStrategy, { - provide: LegacySchoolService, - useValue: createMock(), + provide: AutoSchoolIdStrategy, + useValue: createMock(), }, { - provide: CourseService, - useValue: createMock(), + provide: AutoSchoolNumberStrategy, + useValue: createMock(), + }, + { + provide: AutoContextIdStrategy, + useValue: createMock(), + }, + { + provide: AutoContextNameStrategy, + useValue: createMock(), }, ], }).compile(); diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/basic-tool-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.ts similarity index 100% rename from apps/server/src/modules/tool/tool-launch/service/strategy/basic-tool-launch.strategy.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/basic-tool-launch.strategy.ts diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/index.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/index.ts similarity index 100% rename from apps/server/src/modules/tool/tool-launch/service/strategy/index.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/index.ts diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/lti11-tool-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts similarity index 96% rename from apps/server/src/modules/tool/tool-launch/service/strategy/lti11-tool-launch.strategy.spec.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts index 5113ff3cc76..ee2932cd535 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/lti11-tool-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.spec.ts @@ -1,4 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { PseudonymService } from '@modules/pseudonym/service'; +import { UserService } from '@modules/user'; import { InternalServerErrorException, UnprocessableEntityException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { Pseudonym, RoleName, UserDO } from '@shared/domain'; @@ -9,10 +11,6 @@ import { userDoFactory, } from '@shared/testing'; import { pseudonymFactory } from '@shared/testing/factory/domainobject/pseudonym.factory'; -import { CourseService } from '@modules/learnroom/service'; -import { LegacySchoolService } from '@modules/legacy-school'; -import { PseudonymService } from '@modules/pseudonym/service'; -import { UserService } from '@modules/user'; import { ObjectId } from 'bson'; import { Authorization } from 'oauth-1.0a'; import { LtiMessageType, LtiPrivacyPermission, LtiRole, ToolContextType } from '../../../common/enum'; @@ -20,6 +18,12 @@ import { ContextExternalTool } from '../../../context-external-tool/domain'; import { ExternalTool } from '../../../external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { LaunchRequestMethod, PropertyData, PropertyLocation } from '../../types'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from '../auto-parameter-strategy'; import { Lti11EncryptionService } from '../lti11-encryption.service'; import { Lti11ToolLaunchStrategy } from './lti11-tool-launch.strategy'; import { IToolLaunchParams } from './tool-launch-params.interface'; @@ -32,7 +36,7 @@ describe('Lti11ToolLaunchStrategy', () => { let pseudonymService: DeepMocked; let lti11EncryptionService: DeepMocked; - beforeEach(async () => { + beforeAll(async () => { module = await Test.createTestingModule({ providers: [ Lti11ToolLaunchStrategy, @@ -49,12 +53,20 @@ describe('Lti11ToolLaunchStrategy', () => { useValue: createMock(), }, { - provide: LegacySchoolService, - useValue: createMock(), + provide: AutoSchoolIdStrategy, + useValue: createMock(), + }, + { + provide: AutoSchoolNumberStrategy, + useValue: createMock(), + }, + { + provide: AutoContextIdStrategy, + useValue: createMock(), }, { - provide: CourseService, - useValue: createMock(), + provide: AutoContextNameStrategy, + useValue: createMock(), }, ], }).compile(); diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/lti11-tool-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts similarity index 92% rename from apps/server/src/modules/tool/tool-launch/service/strategy/lti11-tool-launch.strategy.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts index 09d04e388f3..09516395234 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/lti11-tool-launch.strategy.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/lti11-tool-launch.strategy.ts @@ -1,15 +1,19 @@ +import { PseudonymService } from '@modules/pseudonym/service'; +import { UserService } from '@modules/user'; import { Injectable, InternalServerErrorException, UnprocessableEntityException } from '@nestjs/common'; import { EntityId, LtiPrivacyPermission, Pseudonym, RoleName, UserDO } from '@shared/domain'; import { RoleReference } from '@shared/domain/domainobject'; -import { CourseService } from '@modules/learnroom/service'; -import { LegacySchoolService } from '@modules/legacy-school'; -import { PseudonymService } from '@modules/pseudonym/service'; -import { UserService } from '@modules/user'; import { Authorization } from 'oauth-1.0a'; import { LtiRole } from '../../../common/enum'; import { ExternalTool } from '../../../external-tool/domain'; import { LtiRoleMapper } from '../../mapper'; import { AuthenticationValues, LaunchRequestMethod, PropertyData, PropertyLocation } from '../../types'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from '../auto-parameter-strategy'; import { Lti11EncryptionService } from '../lti11-encryption.service'; import { AbstractLaunchStrategy } from './abstract-launch.strategy'; import { IToolLaunchParams } from './tool-launch-params.interface'; @@ -20,10 +24,12 @@ export class Lti11ToolLaunchStrategy extends AbstractLaunchStrategy { private readonly userService: UserService, private readonly pseudonymService: PseudonymService, private readonly lti11EncryptionService: Lti11EncryptionService, - schoolService: LegacySchoolService, - courseService: CourseService + autoSchoolIdStrategy: AutoSchoolIdStrategy, + autoSchoolNumberStrategy: AutoSchoolNumberStrategy, + autoContextIdStrategy: AutoContextIdStrategy, + autoContextNameStrategy: AutoContextNameStrategy ) { - super(schoolService, courseService); + super(autoSchoolIdStrategy, autoSchoolNumberStrategy, autoContextIdStrategy, autoContextNameStrategy); } // eslint-disable-next-line @typescript-eslint/no-unused-vars diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/oauth2-tool-launch.strategy.spec.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts similarity index 80% rename from apps/server/src/modules/tool/tool-launch/service/strategy/oauth2-tool-launch.strategy.spec.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts index bd97fafde71..8f81a1d0eb2 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/oauth2-tool-launch.strategy.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.spec.ts @@ -1,12 +1,16 @@ import { createMock } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; import { contextExternalToolFactory, externalToolFactory, schoolExternalToolFactory } from '@shared/testing'; -import { CourseService } from '@modules/learnroom/service'; -import { LegacySchoolService } from '@modules/legacy-school'; import { ContextExternalTool } from '../../../context-external-tool/domain'; import { ExternalTool } from '../../../external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; import { LaunchRequestMethod, PropertyData } from '../../types'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from '../auto-parameter-strategy'; import { OAuth2ToolLaunchStrategy } from './oauth2-tool-launch.strategy'; import { IToolLaunchParams } from './tool-launch-params.interface'; @@ -14,17 +18,25 @@ describe('OAuth2ToolLaunchStrategy', () => { let module: TestingModule; let strategy: OAuth2ToolLaunchStrategy; - beforeEach(async () => { + beforeAll(async () => { module = await Test.createTestingModule({ providers: [ OAuth2ToolLaunchStrategy, { - provide: LegacySchoolService, - useValue: createMock(), + provide: AutoSchoolIdStrategy, + useValue: createMock(), }, { - provide: CourseService, - useValue: createMock(), + provide: AutoSchoolNumberStrategy, + useValue: createMock(), + }, + { + provide: AutoContextIdStrategy, + useValue: createMock(), + }, + { + provide: AutoContextNameStrategy, + useValue: createMock(), }, ], }).compile(); diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/oauth2-tool-launch.strategy.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.ts similarity index 100% rename from apps/server/src/modules/tool/tool-launch/service/strategy/oauth2-tool-launch.strategy.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/oauth2-tool-launch.strategy.ts diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/tool-launch-params.interface.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/tool-launch-params.interface.ts similarity index 100% rename from apps/server/src/modules/tool/tool-launch/service/strategy/tool-launch-params.interface.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/tool-launch-params.interface.ts index a6d1b75d9cf..24e368476f5 100644 --- a/apps/server/src/modules/tool/tool-launch/service/strategy/tool-launch-params.interface.ts +++ b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/tool-launch-params.interface.ts @@ -1,6 +1,6 @@ +import { ContextExternalTool } from '../../../context-external-tool/domain'; import { ExternalTool } from '../../../external-tool/domain'; import { SchoolExternalTool } from '../../../school-external-tool/domain'; -import { ContextExternalTool } from '../../../context-external-tool/domain'; export interface IToolLaunchParams { externalTool: ExternalTool; diff --git a/apps/server/src/modules/tool/tool-launch/service/strategy/tool-launch-strategy.interface.ts b/apps/server/src/modules/tool/tool-launch/service/launch-strategy/tool-launch-strategy.interface.ts similarity index 100% rename from apps/server/src/modules/tool/tool-launch/service/strategy/tool-launch-strategy.interface.ts rename to apps/server/src/modules/tool/tool-launch/service/launch-strategy/tool-launch-strategy.interface.ts diff --git a/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.spec.ts b/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.spec.ts index 3330b0c9f0e..e4f9eaa6113 100644 --- a/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.spec.ts +++ b/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.spec.ts @@ -21,7 +21,7 @@ import { IToolLaunchParams, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy, -} from './strategy'; +} from './launch-strategy'; import { ToolLaunchService } from './tool-launch.service'; describe('ToolLaunchService', () => { diff --git a/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.ts b/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.ts index 46d2efdeb70..abb5598796f 100644 --- a/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.ts +++ b/apps/server/src/modules/tool/tool-launch/service/tool-launch.service.ts @@ -15,7 +15,7 @@ import { IToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy, -} from './strategy'; +} from './launch-strategy'; @Injectable() export class ToolLaunchService { diff --git a/apps/server/src/modules/tool/tool-launch/tool-launch.module.ts b/apps/server/src/modules/tool/tool-launch/tool-launch.module.ts index 4ae6a3a38a5..b8ff7623586 100644 --- a/apps/server/src/modules/tool/tool-launch/tool-launch.module.ts +++ b/apps/server/src/modules/tool/tool-launch/tool-launch.module.ts @@ -1,14 +1,21 @@ -import { Module, forwardRef } from '@nestjs/common'; +import { BoardModule } from '@modules/board'; import { LearnroomModule } from '@modules/learnroom'; import { LegacySchoolModule } from '@modules/legacy-school'; import { PseudonymModule } from '@modules/pseudonym'; import { UserModule } from '@modules/user'; +import { forwardRef, Module } from '@nestjs/common'; import { CommonToolModule } from '../common'; import { ContextExternalToolModule } from '../context-external-tool'; import { ExternalToolModule } from '../external-tool'; import { SchoolExternalToolModule } from '../school-external-tool'; import { Lti11EncryptionService, ToolLaunchService } from './service'; -import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy } from './service/strategy'; +import { + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, +} from './service/auto-parameter-strategy'; +import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy } from './service/launch-strategy'; @Module({ imports: [ @@ -20,13 +27,18 @@ import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrat UserModule, forwardRef(() => PseudonymModule), // i do not like this solution, the root problem is on other place but not detectable for me LearnroomModule, + BoardModule, ], providers: [ ToolLaunchService, + Lti11EncryptionService, BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrategy, - Lti11EncryptionService, + AutoContextIdStrategy, + AutoContextNameStrategy, + AutoSchoolIdStrategy, + AutoSchoolNumberStrategy, ], exports: [ToolLaunchService], }) diff --git a/backup/setup/external_tools.json b/backup/setup/external_tools.json index 22a582f9a09..2a3fd937a37 100644 --- a/backup/setup/external_tools.json +++ b/backup/setup/external_tools.json @@ -76,9 +76,9 @@ } }, "name": "LTI Test Tool", - "url": "https://www.tsugi.org/lti-test/tool.php", + "url": "https://saltire.lti.app", "config_type": "lti11", - "config_baseUrl": "https://www.tsugi.org/lti-test/tool.php", + "config_baseUrl": "https://saltire.lti.app/tool", "config_key": "12345", "config_secret": "secret", "config_lti_message_type": "basic-lti-launch-request", From f38c0be54871957dd99dfbeff3fa5a8abe87dae7 Mon Sep 17 00:00:00 2001 From: agnisa-cap Date: Fri, 3 Nov 2023 09:00:24 +0100 Subject: [PATCH 7/8] N21-1376 fixes removal of provisioned groups (#4518) --- .../strategy/oidc/oidc.strategy.spec.ts | 41 +++++++++++++++++++ .../strategy/oidc/oidc.strategy.ts | 16 ++++---- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.spec.ts b/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.spec.ts index 2b838159524..e4f50429d0f 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.spec.ts @@ -269,5 +269,46 @@ describe('OidcStrategy', () => { expect(oidcProvisioningService.provisionExternalGroup).not.toHaveBeenCalled(); }); }); + + describe('when group data is not provided', () => { + const setup = () => { + Configuration.set('FEATURE_SANIS_GROUP_PROVISIONING_ENABLED', true); + + const externalUserId = 'externalUserId'; + const oauthData: OauthDataDto = new OauthDataDto({ + system: new ProvisioningSystemDto({ + systemId: 'systemId', + provisioningStrategy: SystemProvisioningStrategy.OIDC, + }), + externalUser: new ExternalUserDto({ + externalId: externalUserId, + }), + externalGroups: undefined, + }); + + const user: UserDO = userDoFactory.withRoles([{ id: 'roleId', name: RoleName.USER }]).build({ + externalId: externalUserId, + }); + + oidcProvisioningService.provisionExternalUser.mockResolvedValue(user); + + return { + externalUserId, + oauthData, + }; + }; + + it('should remove external groups and affiliation', async () => { + const { externalUserId, oauthData } = setup(); + + await strategy.apply(oauthData); + + expect(oidcProvisioningService.removeExternalGroupsAndAffiliation).toHaveBeenCalledWith( + externalUserId, + [], + oauthData.system.systemId + ); + }); + }); }); }); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.ts b/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.ts index 7804f2190f9..4cb3e920da6 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/oidc.strategy.ts @@ -23,18 +23,20 @@ export abstract class OidcProvisioningStrategy extends ProvisioningStrategy { school?.id ); - if (Configuration.get('FEATURE_SANIS_GROUP_PROVISIONING_ENABLED') && data.externalGroups) { + if (Configuration.get('FEATURE_SANIS_GROUP_PROVISIONING_ENABLED')) { await this.oidcProvisioningService.removeExternalGroupsAndAffiliation( data.externalUser.externalId, - data.externalGroups, + data.externalGroups ?? [], data.system.systemId ); - await Promise.all( - data.externalGroups.map((externalGroup) => - this.oidcProvisioningService.provisionExternalGroup(externalGroup, data.system.systemId) - ) - ); + if (data.externalGroups) { + await Promise.all( + data.externalGroups.map((externalGroup) => + this.oidcProvisioningService.provisionExternalGroup(externalGroup, data.system.systemId) + ) + ); + } } return new ProvisioningDto({ externalUserId: user.externalId || data.externalUser.externalId }); From f4587465a67f140a02df0d71cb54206a8e2a9b44 Mon Sep 17 00:00:00 2001 From: Max <53796487+dyedwiper@users.noreply.github.com> Date: Fri, 3 Nov 2023 11:45:17 +0100 Subject: [PATCH 8/8] BC-5712 Make test workflow manually runnable (#4519) --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cbf52acce55..2b087f1bc51 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,6 +5,7 @@ on: branches: [ main ] pull_request: branches: [ main ] + workflow_dispatch: permissions: contents: read