From c0e753541a4ba542755bc0162618f30c0e3097b7 Mon Sep 17 00:00:00 2001 From: Arne Gnisa Date: Mon, 13 May 2024 10:40:48 +0200 Subject: [PATCH 1/4] N21-1968 fixes missing secrets --- .../templates/amqp-files-deployment.yml.j2 | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ansible/roles/schulcloud-server-core/templates/amqp-files-deployment.yml.j2 b/ansible/roles/schulcloud-server-core/templates/amqp-files-deployment.yml.j2 index c8232c7bd04..61327992202 100644 --- a/ansible/roles/schulcloud-server-core/templates/amqp-files-deployment.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/amqp-files-deployment.yml.j2 @@ -52,6 +52,8 @@ spec: name: amqp-files-configmap - secretRef: name: amqp-files-secret + - secretRef: + name: api-secret command: ['npm', 'run', 'nest:start:files-storage-amqp:prod'] resources: limits: @@ -97,5 +99,5 @@ spec: - amqp-files topologyKey: "topology.kubernetes.io/zone" {% endif %} - - + + From 7a23f6958ad85f0ea6eaf1b95d5f1ea41d08aafa Mon Sep 17 00:00:00 2001 From: Arne Gnisa Date: Mon, 13 May 2024 11:21:40 +0200 Subject: [PATCH 2/4] N21-1968 removes required of schulconnex props --- .../templates/amqp-files-deployment.yml.j2 | 2 -- config/default.schema.json | 6 ------ 2 files changed, 8 deletions(-) diff --git a/ansible/roles/schulcloud-server-core/templates/amqp-files-deployment.yml.j2 b/ansible/roles/schulcloud-server-core/templates/amqp-files-deployment.yml.j2 index 61327992202..e5d99c0b49f 100644 --- a/ansible/roles/schulcloud-server-core/templates/amqp-files-deployment.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/amqp-files-deployment.yml.j2 @@ -52,8 +52,6 @@ spec: name: amqp-files-configmap - secretRef: name: amqp-files-secret - - secretRef: - name: api-secret command: ['npm', 'run', 'nest:start:files-storage-amqp:prod'] resources: limits: diff --git a/config/default.schema.json b/config/default.schema.json index 3fe3385dcde..4f646d97507 100644 --- a/config/default.schema.json +++ b/config/default.schema.json @@ -1606,12 +1606,6 @@ "SCHULCONNEX_CLIENT": { "type": "object", "description": "Configuration of the schulcloud's schulconnex client.", - "required": [ - "API_URL", - "TOKEN_ENDPOINT", - "CLIENT_ID", - "CLIENT_SECRET" - ], "properties": { "API_URL": { "type": "string", From e655c679ad757b84bf18f3d8b52c07d34d69d3db Mon Sep 17 00:00:00 2001 From: Arne Gnisa Date: Tue, 14 May 2024 10:23:43 +0200 Subject: [PATCH 3/4] N21-1968 changes schulconnex configs from config service and log message if their are missing --- .../schulconnex-client.module.ts | 22 ++++++++++++--- .../schulconnex-rest-client-options.ts | 8 +++--- .../schulconnex-rest-client.ts | 16 +++++++---- .../modules/idp-console/idp-console.module.ts | 27 +++++++------------ .../provisioning/provisioning.module.ts | 9 +------ .../src/modules/server/server.config.ts | 20 ++++++++++++-- .../src/modules/server/server.module.ts | 10 ++----- .../modules/user-import/user-import.module.ts | 9 +------ config/development.json | 4 +-- 9 files changed, 66 insertions(+), 59 deletions(-) diff --git a/apps/server/src/infra/schulconnex-client/schulconnex-client.module.ts b/apps/server/src/infra/schulconnex-client/schulconnex-client.module.ts index 36aae0e119b..11a6c3c8278 100644 --- a/apps/server/src/infra/schulconnex-client/schulconnex-client.module.ts +++ b/apps/server/src/infra/schulconnex-client/schulconnex-client.module.ts @@ -1,13 +1,14 @@ import { OauthAdapterService } from '@modules/oauth/service'; import { HttpModule, HttpService } from '@nestjs/axios'; import { DynamicModule, Module } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; import { Logger, LoggerModule } from '@src/core/logger'; import { SchulconnexRestClient } from './schulconnex-rest-client'; import { SchulconnexRestClientOptions } from './schulconnex-rest-client-options'; @Module({}) export class SchulconnexClientModule { - static register(options: SchulconnexRestClientOptions): DynamicModule { + static registerAsync(): DynamicModule { return { imports: [HttpModule, LoggerModule], module: SchulconnexClientModule, @@ -15,9 +16,22 @@ export class SchulconnexClientModule { OauthAdapterService, { provide: SchulconnexRestClient, - useFactory: (httpService: HttpService, oauthAdapterService: OauthAdapterService, logger: Logger) => - new SchulconnexRestClient(options, httpService, oauthAdapterService, logger), - inject: [HttpService, OauthAdapterService, Logger], + useFactory: ( + httpService: HttpService, + oauthAdapterService: OauthAdapterService, + logger: Logger, + configService: ConfigService + ) => { + const options: SchulconnexRestClientOptions = { + apiUrl: configService.get('SCHULCONNEX_CLIENT__API_URL'), + tokenEndpoint: configService.get('SCHULCONNEX_CLIENT__TOKEN_ENDPOINT'), + clientId: configService.get('SCHULCONNEX_CLIENT__CLIENT_ID'), + clientSecret: configService.get('SCHULCONNEX_CLIENT__CLIENT_SECRET'), + personenInfoTimeoutInMs: configService.get('SCHULCONNEX_CLIENT__PERSONEN_INFO_TIMEOUT_IN_MS'), + }; + return new SchulconnexRestClient(options, httpService, oauthAdapterService, logger); + }, + inject: [HttpService, OauthAdapterService, Logger, ConfigService], }, ], exports: [SchulconnexRestClient], diff --git a/apps/server/src/infra/schulconnex-client/schulconnex-rest-client-options.ts b/apps/server/src/infra/schulconnex-client/schulconnex-rest-client-options.ts index d2f8ebb3a1a..9f47b1d5694 100644 --- a/apps/server/src/infra/schulconnex-client/schulconnex-rest-client-options.ts +++ b/apps/server/src/infra/schulconnex-client/schulconnex-rest-client-options.ts @@ -1,11 +1,11 @@ export interface SchulconnexRestClientOptions { - apiUrl: string; + apiUrl?: string; - tokenEndpoint: string; + tokenEndpoint?: string; - clientId: string; + clientId?: string; - clientSecret: string; + clientSecret?: string; personenInfoTimeoutInMs?: number; } diff --git a/apps/server/src/infra/schulconnex-client/schulconnex-rest-client.ts b/apps/server/src/infra/schulconnex-client/schulconnex-rest-client.ts index d96f2cd47bf..f1b8835bedb 100644 --- a/apps/server/src/infra/schulconnex-client/schulconnex-rest-client.ts +++ b/apps/server/src/infra/schulconnex-client/schulconnex-rest-client.ts @@ -22,7 +22,7 @@ export class SchulconnexRestClient implements SchulconnexApiInterface { private readonly logger: Logger ) { this.checkOptions(); - this.SCHULCONNEX_API_BASE_URL = options.apiUrl; + this.SCHULCONNEX_API_BASE_URL = options.apiUrl || ''; } // TODO: N21-1678 use this in provisioning module @@ -63,10 +63,12 @@ export class SchulconnexRestClient implements SchulconnexApiInterface { return response; } - private checkOptions(): void { + private checkOptions(): boolean { if (!this.options.apiUrl || !this.options.clientId || !this.options.clientSecret || !this.options.tokenEndpoint) { this.logger.debug(new SchulconnexConfigurationMissingLoggable()); + return false; } + return true; } private async getRequest(url: URL, accessToken: string, timeout?: number): Promise { @@ -86,13 +88,17 @@ export class SchulconnexRestClient implements SchulconnexApiInterface { private async requestClientCredentialToken(): Promise { const { tokenEndpoint, clientId, clientSecret } = this.options; + if (!this.checkOptions()) { + return Promise.reject(new Error('Missing configuration for SchulconnexRestClient')); + } + const payload: ClientCredentialsGrantTokenRequest = new ClientCredentialsGrantTokenRequest({ - client_id: clientId, - client_secret: clientSecret, + client_id: clientId ?? '', + client_secret: clientSecret ?? '', grant_type: OAuthGrantType.CLIENT_CREDENTIALS_GRANT, }); - const tokenDto: OAuthTokenDto = await this.oauthAdapterService.sendTokenRequest(tokenEndpoint, payload); + const tokenDto: OAuthTokenDto = await this.oauthAdapterService.sendTokenRequest(tokenEndpoint ?? '', payload); return tokenDto; } diff --git a/apps/server/src/modules/idp-console/idp-console.module.ts b/apps/server/src/modules/idp-console/idp-console.module.ts index f2be287c729..d9ad00aa321 100644 --- a/apps/server/src/modules/idp-console/idp-console.module.ts +++ b/apps/server/src/modules/idp-console/idp-console.module.ts @@ -1,30 +1,23 @@ -import { Module } from '@nestjs/common'; -import { ConfigModule } from '@nestjs/config'; -import { Configuration } from '@hpi-schul-cloud/commons/lib'; +import { ConsoleWriterModule } from '@infra/console'; +import { RabbitMQWrapperModule } from '@infra/rabbitmq'; import { SchulconnexClientModule } from '@infra/schulconnex-client'; -import { SynchronizationEntity, SynchronizationModule } from '@modules/synchronization'; -import { defaultMikroOrmOptions } from '@modules/server'; -import { DB_PASSWORD, DB_URL, DB_USERNAME } from '@src/config'; -import { ALL_ENTITIES } from '@shared/domain/entity'; import { MikroOrmModule } from '@mikro-orm/nestjs'; +import { defaultMikroOrmOptions } from '@modules/server'; +import { SynchronizationEntity, SynchronizationModule } from '@modules/synchronization'; import { UserModule } from '@modules/user'; +import { Module } from '@nestjs/common'; +import { ConfigModule } from '@nestjs/config'; +import { ALL_ENTITIES } from '@shared/domain/entity'; +import { DB_PASSWORD, DB_URL, DB_USERNAME } from '@src/config'; import { LoggerModule } from '@src/core/logger'; -import { RabbitMQWrapperModule } from '@infra/rabbitmq'; -import { ConsoleWriterModule } from '@infra/console'; import { ConsoleModule } from 'nestjs-console'; -import { SynchronizationUc } from './uc'; import { IdpSyncConsole } from './idp-sync-console'; +import { SynchronizationUc } from './uc'; @Module({ imports: [ ConfigModule.forRoot({ isGlobal: true }), - SchulconnexClientModule.register({ - apiUrl: Configuration.get('SCHULCONNEX_CLIENT__API_URL') as string, - tokenEndpoint: Configuration.get('SCHULCONNEX_CLIENT__TOKEN_ENDPOINT') as string, - clientId: Configuration.get('SCHULCONNEX_CLIENT__CLIENT_ID') as string, - clientSecret: Configuration.get('SCHULCONNEX_CLIENT__CLIENT_SECRET') as string, - personenInfoTimeoutInMs: Configuration.get('SCHULCONNEX_CLIENT__PERSONEN_INFO_TIMEOUT_IN_MS') as number, - }), + SchulconnexClientModule.registerAsync(), SynchronizationModule, MikroOrmModule.forRoot({ ...defaultMikroOrmOptions, diff --git a/apps/server/src/modules/provisioning/provisioning.module.ts b/apps/server/src/modules/provisioning/provisioning.module.ts index 953ba669671..11dd7cfeb89 100644 --- a/apps/server/src/modules/provisioning/provisioning.module.ts +++ b/apps/server/src/modules/provisioning/provisioning.module.ts @@ -1,4 +1,3 @@ -import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { AccountModule } from '@modules/account'; import { GroupModule } from '@modules/group'; import { LearnroomModule } from '@modules/learnroom'; @@ -39,13 +38,7 @@ import { LoggerModule, GroupModule, LearnroomModule, - SchulconnexClientModule.register({ - apiUrl: Configuration.get('SCHULCONNEX_CLIENT__API_URL') as string, - tokenEndpoint: Configuration.get('SCHULCONNEX_CLIENT__TOKEN_ENDPOINT') as string, - clientId: Configuration.get('SCHULCONNEX_CLIENT__CLIENT_ID') as string, - clientSecret: Configuration.get('SCHULCONNEX_CLIENT__CLIENT_SECRET') as string, - personenInfoTimeoutInMs: Configuration.get('SCHULCONNEX_CLIENT__PERSONEN_INFO_TIMEOUT_IN_MS') as number, - }), + SchulconnexClientModule.registerAsync(), UserLicenseModule, ], providers: [ diff --git a/apps/server/src/modules/server/server.config.ts b/apps/server/src/modules/server/server.config.ts index eac92886144..404cba4d8b1 100644 --- a/apps/server/src/modules/server/server.config.ts +++ b/apps/server/src/modules/server/server.config.ts @@ -2,6 +2,7 @@ import { Configuration } from '@hpi-schul-cloud/commons'; import type { IdentityManagementConfig } from '@infra/identity-management'; import type { SchulconnexClientConfig } from '@infra/schulconnex-client'; import type { AccountConfig } from '@modules/account'; +import { AlertConfig } from '@modules/alert'; import type { AuthenticationConfig, XApiKeyConfig } from '@modules/authentication'; import type { BoardConfig } from '@modules/board'; import type { MediaBoardConfig } from '@modules/board/media-board.config'; @@ -21,10 +22,9 @@ import { type IUserImportFeatures, UserImportConfiguration } from '@modules/user import type { UserLoginMigrationConfig } from '@modules/user-login-migration'; import { type IVideoConferenceSettings, VideoConferenceConfiguration } from '@modules/video-conference'; import { LanguageType } from '@shared/domain/interface'; +import { SchulcloudTheme } from '@shared/domain/types'; import type { CoreModuleConfig } from '@src/core'; import type { MailConfig } from '@src/infra/mail/interfaces/mail-config'; -import { AlertConfig } from '@modules/alert'; -import { SchulcloudTheme } from '@shared/domain/types'; import { Timezone } from './types/timezone.enum'; export enum NodeEnvType { @@ -112,6 +112,10 @@ export interface ServerConfig I18N__DEFAULT_TIMEZONE: Timezone; BOARD_COLLABORATION_URI: string; FEATURE_NEW_LAYOUT_ENABLED: boolean; + SCHULCONNEX_CLIENT__API_URL: string | undefined; + SCHULCONNEX_CLIENT__TOKEN_ENDPOINT: string | undefined; + SCHULCONNEX_CLIENT__CLIENT_ID: string | undefined; + SCHULCONNEX_CLIENT__CLIENT_SECRET: string | undefined; } const config: ServerConfig = { @@ -223,6 +227,18 @@ const config: ServerConfig = { I18N__DEFAULT_LANGUAGE: Configuration.get('I18N__DEFAULT_LANGUAGE') as unknown as LanguageType, I18N__FALLBACK_LANGUAGE: Configuration.get('I18N__FALLBACK_LANGUAGE') as unknown as LanguageType, I18N__DEFAULT_TIMEZONE: Configuration.get('I18N__DEFAULT_TIMEZONE') as Timezone, + SCHULCONNEX_CLIENT__API_URL: Configuration.has('SCHULCONNEX_CLIENT__API_URL') + ? (Configuration.get('SCHULCONNEX_CLIENT__API_URL') as string) + : undefined, + SCHULCONNEX_CLIENT__TOKEN_ENDPOINT: Configuration.has('SCHULCONNEX_CLIENT__TOKEN_ENDPOINT') + ? (Configuration.get('SCHULCONNEX_CLIENT__TOKEN_ENDPOINT') as string) + : undefined, + SCHULCONNEX_CLIENT__CLIENT_ID: Configuration.has('SCHULCONNEX_CLIENT__CLIENT_ID') + ? (Configuration.get('SCHULCONNEX_CLIENT__CLIENT_ID') as string) + : undefined, + SCHULCONNEX_CLIENT__CLIENT_SECRET: Configuration.has('SCHULCONNEX_CLIENT__CLIENT_SECRET') + ? (Configuration.get('SCHULCONNEX_CLIENT__CLIENT_SECRET') as string) + : undefined, SCHULCONNEX_CLIENT__PERSONEN_INFO_TIMEOUT_IN_MS: Configuration.get( 'SCHULCONNEX_CLIENT__PERSONEN_INFO_TIMEOUT_IN_MS' ) as number, diff --git a/apps/server/src/modules/server/server.module.ts b/apps/server/src/modules/server/server.module.ts index 5eff240c233..d7b533f5be9 100644 --- a/apps/server/src/modules/server/server.module.ts +++ b/apps/server/src/modules/server/server.module.ts @@ -6,6 +6,7 @@ import { SchulconnexClientModule } from '@infra/schulconnex-client'; import { Dictionary, IPrimaryKey } from '@mikro-orm/core'; import { MikroOrmModule, MikroOrmModuleSyncOptions } from '@mikro-orm/nestjs'; import { AccountApiModule } from '@modules/account/account-api.module'; +import { AlertModule } from '@modules/alert/alert.module'; import { AuthenticationApiModule } from '@modules/authentication/authentication-api.module'; import { BoardApiModule } from '@modules/board/board-api.module'; import { MediaBoardApiModule } from '@modules/board/media-board-api.module'; @@ -40,7 +41,6 @@ import { ALL_ENTITIES } from '@shared/domain/entity'; import { createConfigModuleOptions, DB_PASSWORD, DB_URL, DB_USERNAME } from '@src/config'; import { CoreModule } from '@src/core'; import { LoggerModule } from '@src/core/logger'; -import { AlertModule } from '@modules/alert/alert.module'; import { UserLicenseModule } from '../user-license'; import { ServerConfigController, ServerController, ServerUc } from './api'; import { SERVER_CONFIG_TOKEN, serverConfig } from './server.config'; @@ -58,13 +58,7 @@ const serverModules = [ NewsModule, UserApiModule, UsersAdminApiModule, - SchulconnexClientModule.register({ - apiUrl: Configuration.get('SCHULCONNEX_CLIENT__API_URL') as string, - tokenEndpoint: Configuration.get('SCHULCONNEX_CLIENT__TOKEN_ENDPOINT') as string, - clientId: Configuration.get('SCHULCONNEX_CLIENT__CLIENT_ID') as string, - clientSecret: Configuration.get('SCHULCONNEX_CLIENT__CLIENT_SECRET') as string, - personenInfoTimeoutInMs: Configuration.get('SCHULCONNEX_CLIENT__PERSONEN_INFO_TIMEOUT_IN_MS') as number, - }), + SchulconnexClientModule.registerAsync(), ImportUserModule, UserImportConfigModule, LearnroomApiModule, diff --git a/apps/server/src/modules/user-import/user-import.module.ts b/apps/server/src/modules/user-import/user-import.module.ts index 6d56bf0b4b9..7caed266954 100644 --- a/apps/server/src/modules/user-import/user-import.module.ts +++ b/apps/server/src/modules/user-import/user-import.module.ts @@ -1,4 +1,3 @@ -import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { SchulconnexClientModule } from '@infra/schulconnex-client'; import { AccountModule } from '@modules/account'; import { AuthorizationModule } from '@modules/authorization'; @@ -25,13 +24,7 @@ import { UserImportConfigModule } from './user-import-config.module'; HttpModule, UserModule, OauthModule, - SchulconnexClientModule.register({ - apiUrl: Configuration.get('SCHULCONNEX_CLIENT__API_URL') as string, - tokenEndpoint: Configuration.get('SCHULCONNEX_CLIENT__TOKEN_ENDPOINT') as string, - clientId: Configuration.get('SCHULCONNEX_CLIENT__CLIENT_ID') as string, - clientSecret: Configuration.get('SCHULCONNEX_CLIENT__CLIENT_SECRET') as string, - personenInfoTimeoutInMs: Configuration.get('SCHULCONNEX_CLIENT__PERSONEN_INFO_TIMEOUT_IN_MS') as number, - }), + SchulconnexClientModule.registerAsync(), UserLoginMigrationModule, ], controllers: [ImportUserController], diff --git a/config/development.json b/config/development.json index 7387a5dacaa..956966a8d76 100644 --- a/config/development.json +++ b/config/development.json @@ -85,9 +85,7 @@ "FEATURE_BOARD_LAYOUT_ENABLED": true, "SCHULCONNEX_CLIENT": { "API_URL": "http://localhost:8888/v1/", - "TOKEN_ENDPOINT": "http://localhost:8888/realms/SANIS/protocol/openid-connect/token", - "CLIENT_ID": "schulcloud", - "CLIENT_SECRET": "secret" + "TOKEN_ENDPOINT": "http://localhost:8888/realms/SANIS/protocol/openid-connect/token" }, "ETHERPAD_URI": "http://localhost:9001/api/1/", "ETHERPAD": { From 27c2ecd876b17fa36d31ef7fdf3c32b0f75b2120 Mon Sep 17 00:00:00 2001 From: Arne Gnisa Date: Tue, 14 May 2024 10:49:08 +0200 Subject: [PATCH 4/4] N21-1968 adds test --- .../schulconnex-rest-client.spec.ts | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/apps/server/src/infra/schulconnex-client/schulconnex-rest-client.spec.ts b/apps/server/src/infra/schulconnex-client/schulconnex-rest-client.spec.ts index 7b6f15c3342..5a14e88b03c 100644 --- a/apps/server/src/infra/schulconnex-client/schulconnex-rest-client.spec.ts +++ b/apps/server/src/infra/schulconnex-client/schulconnex-rest-client.spec.ts @@ -41,8 +41,8 @@ describe(SchulconnexRestClient.name, () => { const setup = () => { const badOptions: SchulconnexRestClientOptions = { apiUrl: '', - clientId: '', - clientSecret: '', + clientId: undefined, + clientSecret: undefined, tokenEndpoint: '', }; return { @@ -58,6 +58,16 @@ describe(SchulconnexRestClient.name, () => { expect(logger.debug).toHaveBeenCalledWith(new SchulconnexConfigurationMissingLoggable()); }); + + it('should reject promise if configuration is missing', async () => { + const { badOptions } = setup(); + + const badOptionsClient = new SchulconnexRestClient(badOptions, httpService, oauthAdapterService, logger); + + await expect(badOptionsClient.getPersonenInfo({})).rejects.toThrow( + 'Missing configuration for SchulconnexRestClient' + ); + }); }); }); @@ -80,7 +90,7 @@ describe(SchulconnexRestClient.name, () => { await client.getPersonInfo(accessToken); - expect(httpService.get).toHaveBeenCalledWith(`${options.apiUrl}/person-info`, { + expect(httpService.get).toHaveBeenCalledWith(`${options.apiUrl ?? ''}/person-info`, { headers: { Authorization: `Bearer ${accessToken}`, 'Accept-Encoding': 'gzip', @@ -163,7 +173,9 @@ describe(SchulconnexRestClient.name, () => { }); expect(httpService.get).toHaveBeenCalledWith( - `${optionsWithTimeout.apiUrl}/personen-info?organisation.id=1234&vollstaendig=personen%2Corganisationen`, + `${ + optionsWithTimeout.apiUrl ?? '' + }/personen-info?organisation.id=1234&vollstaendig=personen%2Corganisationen`, { headers: { Authorization: `Bearer ${tokens.accessToken}`, @@ -202,7 +214,7 @@ describe(SchulconnexRestClient.name, () => { await client.getLizenzInfo(accessToken); - expect(httpService.get).toHaveBeenCalledWith(`${options.apiUrl}/lizenz-info`, { + expect(httpService.get).toHaveBeenCalledWith(`${options.apiUrl ?? ''}/lizenz-info`, { headers: { Authorization: `Bearer ${accessToken}`, 'Accept-Encoding': 'gzip',