From 0a32c99a52ed29178afe2ac0bed22d0ec485c11b Mon Sep 17 00:00:00 2001 From: Martin Schuhmacher <55735359+MartinSchuhmacher@users.noreply.github.com> Date: Fri, 27 Oct 2023 14:59:05 +0200 Subject: [PATCH 1/4] Bump crypto-js from 4.1.1 to 4.2.0 (#4508) updated-dependencies: - dependency-name: crypto-js (from 4.1.1 to 4.2.0.) - dependency-type: direct:production --- package-lock.json | 14 +++++++------- package.json | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index e36ecdd9873..cd0606b9fde 100644 --- a/package-lock.json +++ b/package-lock.json @@ -68,7 +68,7 @@ "connect-redis": "^6.1.3", "cors": "^2.8.1", "cross-env": "^7.0.0", - "crypto-js": "^4.0.0", + "crypto-js": "^4.2.0", "disposable-email-domains": "^1.0.56", "es6-promisify": "^7.0.0", "express": "^4.14.0", @@ -9069,9 +9069,9 @@ } }, "node_modules/crypto-js": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/crypto-js/-/crypto-js-4.1.1.tgz", - "integrity": "sha512-o2JlM7ydqd3Qk9CA0L4NL6mTzU2sdx96a+oOfPu8Mkl/PK51vSyoi8/rQ8NknZtk44vq15lmhAj9CIAGwgeWKw==" + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/crypto-js/-/crypto-js-4.2.0.tgz", + "integrity": "sha512-KALDyEYgpY+Rlob/iriUtjV6d5Eq+Y191A5g4UqLAi8CyGP9N1+FdVbkc1SxKc2r4YAYqG8JzO2KGL+AizD70Q==" }, "node_modules/css-select": { "version": "5.1.0", @@ -31721,9 +31721,9 @@ "integrity": "sha1-iNf/fsDfuG9xPch7u0LQRNPmxBs=" }, "crypto-js": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/crypto-js/-/crypto-js-4.1.1.tgz", - "integrity": "sha512-o2JlM7ydqd3Qk9CA0L4NL6mTzU2sdx96a+oOfPu8Mkl/PK51vSyoi8/rQ8NknZtk44vq15lmhAj9CIAGwgeWKw==" + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/crypto-js/-/crypto-js-4.2.0.tgz", + "integrity": "sha512-KALDyEYgpY+Rlob/iriUtjV6d5Eq+Y191A5g4UqLAi8CyGP9N1+FdVbkc1SxKc2r4YAYqG8JzO2KGL+AizD70Q==" }, "css-select": { "version": "5.1.0", diff --git a/package.json b/package.json index a82df72904f..45e150f6668 100644 --- a/package.json +++ b/package.json @@ -151,7 +151,7 @@ "connect-redis": "^6.1.3", "cors": "^2.8.1", "cross-env": "^7.0.0", - "crypto-js": "^4.0.0", + "crypto-js": "^4.2.0", "disposable-email-domains": "^1.0.56", "es6-promisify": "^7.0.0", "express": "^4.14.0", From 0cb9d54e58142979c9f7dae4bd72ea1e240eb7a6 Mon Sep 17 00:00:00 2001 From: Sergej Hoffmann <97111299+SevenWaysDP@users.noreply.github.com> Date: Mon, 30 Oct 2023 11:28:30 +0100 Subject: [PATCH 2/4] BC-5489 - For loggables it should possible to pass unknown cause error (#4501) --- .../error/filter/global-error.filter.spec.ts | 104 +++++++++++++++++- .../core/error/filter/global-error.filter.ts | 4 +- 2 files changed, 103 insertions(+), 5 deletions(-) diff --git a/apps/server/src/core/error/filter/global-error.filter.spec.ts b/apps/server/src/core/error/filter/global-error.filter.spec.ts index ca40620515f..c45c13e4bff 100644 --- a/apps/server/src/core/error/filter/global-error.filter.spec.ts +++ b/apps/server/src/core/error/filter/global-error.filter.spec.ts @@ -1,7 +1,7 @@ /* eslint-disable promise/valid-params */ import { NotFound } from '@feathersjs/errors'; import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { ArgumentsHost, BadRequestException, HttpStatus } from '@nestjs/common'; +import { ArgumentsHost, BadRequestException, HttpStatus, InternalServerErrorException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { BusinessError } from '@shared/common'; import { ErrorLogger, ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger'; @@ -9,6 +9,7 @@ import { Response } from 'express'; import util from 'util'; import { ErrorResponse } from '../dto'; import { ErrorLoggable } from '../loggable/error.loggable'; +import { ErrorUtils } from '../utils'; import { GlobalErrorFilter } from './global-error.filter'; class SampleBusinessError extends BusinessError { @@ -42,6 +43,24 @@ class SampleLoggableException extends BadRequestException implements Loggable { } } +class SampleLoggableExceptionWithCause extends InternalServerErrorException implements Loggable { + constructor(private readonly testValue: string, error?: unknown) { + super(ErrorUtils.createHttpExceptionOptions(error)); + } + + getLogMessage(): ErrorLogMessage { + const message: ErrorLogMessage = { + type: 'WITH_CAUSE', + stack: this.stack, + data: { + testValue: this.testValue, + }, + }; + + return message; + } +} + describe('GlobalErrorFilter', () => { let module: TestingModule; let service: GlobalErrorFilter; @@ -304,24 +323,101 @@ describe('GlobalErrorFilter', () => { ).toBeCalledWith(expectedResponse); }); }); + + describe('when error has a cause error', () => { + const setup = () => { + const causeError = new Error('Cause error'); + const error = new SampleLoggableExceptionWithCause('test', causeError); + const expectedResponse = new ErrorResponse( + 'SAMPLE_WITH_CAUSE', + 'Sample With Cause', + 'Sample Loggable Exception With Cause', + HttpStatus.INTERNAL_SERVER_ERROR + ); + + const argumentsHost = setupHttpArgumentsHost(); + + return { error, argumentsHost, expectedResponse }; + }; + + it('should set response status appropriately', () => { + const { error, argumentsHost } = setup(); + + service.catch(error, argumentsHost); + + expect(argumentsHost.switchToHttp().getResponse().status).toBeCalledWith( + HttpStatus.INTERNAL_SERVER_ERROR + ); + }); + + it('should send appropriate error response', () => { + const { error, argumentsHost, expectedResponse } = setup(); + + service.catch(error, argumentsHost); + + expect( + argumentsHost.switchToHttp().getResponse().status(HttpStatus.INTERNAL_SERVER_ERROR).json + ).toBeCalledWith(expectedResponse); + }); + }); }); describe('when context is rmq', () => { + describe('when error is unknown error', () => { + const setup = () => { + const argumentsHost = createMock(); + argumentsHost.getType.mockReturnValueOnce('rmq'); + + const error = new Error(); + + return { error, argumentsHost }; + }; + + it('should return an RpcMessage with the error', () => { + const { error, argumentsHost } = setup(); + + const result = service.catch(error, argumentsHost); + + expect(result).toEqual({ message: undefined, error }); + }); + }); + + describe('when error is a LoggableError', () => { + const setup = () => { + const causeError = new Error('Cause error'); + const error = new SampleLoggableExceptionWithCause('test', causeError); + const argumentsHost = createMock(); + argumentsHost.getType.mockReturnValueOnce('rmq'); + + return { error, argumentsHost }; + }; + + it('should return appropriate error', () => { + const { error, argumentsHost } = setup(); + + const result = service.catch(error, argumentsHost); + + expect(result).toEqual({ message: undefined, error }); + }); + }); + }); + + describe('when context is other than rmq and http', () => { const setup = () => { const argumentsHost = createMock(); - argumentsHost.getType.mockReturnValueOnce('rmq'); + argumentsHost.getType.mockReturnValueOnce('other'); const error = new Error(); return { error, argumentsHost }; }; - it('should return an RpcMessage with the error', () => { + it('should return undefined', () => { const { error, argumentsHost } = setup(); const result = service.catch(error, argumentsHost); - expect(result).toEqual({ message: undefined, error }); + expect(result).toBeUndefined(); }); }); }); diff --git a/apps/server/src/core/error/filter/global-error.filter.ts b/apps/server/src/core/error/filter/global-error.filter.ts index 7e0d1dc3c3f..56760b18dd9 100644 --- a/apps/server/src/core/error/filter/global-error.filter.ts +++ b/apps/server/src/core/error/filter/global-error.filter.ts @@ -24,7 +24,9 @@ export class GlobalErrorFilter implements Exceptio if (contextType === 'http') { this.sendHttpResponse(error, host); - } else if (contextType === 'rmq') { + } + + if (contextType === 'rmq') { return { message: undefined, error }; } } 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 3/4] 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 4/4] 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;