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/3] 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/3] 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/3] 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