From f81f023ddff2194b5154e58a511f271022df33a4 Mon Sep 17 00:00:00 2001 From: sszafGCA <116172610+sszafGCA@users.noreply.github.com> Date: Thu, 9 Nov 2023 16:52:31 +0100 Subject: [PATCH] BC-4895-stop sending emails to non existent thr @schul-cloud.org addresses (#4476) * impl of mail addresses whitelist logic --- .../src/infra/mail/interfaces/mail-config.ts | 3 ++ apps/server/src/infra/mail/mail.module.ts | 3 ++ .../src/infra/mail/mail.service.spec.ts | 49 ++++++++++++++++--- apps/server/src/infra/mail/mail.service.ts | 42 ++++++++++++++-- .../src/modules/server/server.config.ts | 7 ++- config/default.schema.json | 1 + 6 files changed, 95 insertions(+), 10 deletions(-) create mode 100644 apps/server/src/infra/mail/interfaces/mail-config.ts diff --git a/apps/server/src/infra/mail/interfaces/mail-config.ts b/apps/server/src/infra/mail/interfaces/mail-config.ts new file mode 100644 index 00000000000..6dbb0c7864d --- /dev/null +++ b/apps/server/src/infra/mail/interfaces/mail-config.ts @@ -0,0 +1,3 @@ +export interface IMailConfig { + ADDITIONAL_BLACKLISTED_EMAIL_DOMAINS: string[]; +} diff --git a/apps/server/src/infra/mail/mail.module.ts b/apps/server/src/infra/mail/mail.module.ts index 1ca0630c44f..ee6d50d59e7 100644 --- a/apps/server/src/infra/mail/mail.module.ts +++ b/apps/server/src/infra/mail/mail.module.ts @@ -1,5 +1,7 @@ import { Module, DynamicModule } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; import { MailService } from './mail.service'; +import { IMailConfig } from './interfaces/mail-config'; interface MailModuleOptions { exchange: string; @@ -17,6 +19,7 @@ export class MailModule { provide: 'MAIL_SERVICE_OPTIONS', useValue: { exchange: options.exchange, routingKey: options.routingKey }, }, + ConfigService, ], exports: [MailService], }; diff --git a/apps/server/src/infra/mail/mail.service.spec.ts b/apps/server/src/infra/mail/mail.service.spec.ts index 58c0ce9336a..ebc77030252 100644 --- a/apps/server/src/infra/mail/mail.service.spec.ts +++ b/apps/server/src/infra/mail/mail.service.spec.ts @@ -1,7 +1,10 @@ import { AmqpConnection } from '@golevelup/nestjs-rabbitmq'; import { Test, TestingModule } from '@nestjs/testing'; +import { createMock } from '@golevelup/ts-jest'; +import { ConfigService } from '@nestjs/config'; import { Mail } from './mail.interface'; import { MailService } from './mail.service'; +import { IMailConfig } from './interfaces/mail-config'; describe('MailService', () => { let module: TestingModule; @@ -19,6 +22,10 @@ describe('MailService', () => { MailService, { provide: AmqpConnection, useValue: { publish: () => {} } }, { provide: 'MAIL_SERVICE_OPTIONS', useValue: mailServiceOptions }, + { + provide: ConfigService, + useValue: createMock>({ get: () => ['schul-cloud.org', 'example.com'] }), + }, ], }).compile(); @@ -34,13 +41,43 @@ describe('MailService', () => { expect(service).toBeDefined(); }); - it('should send given data to queue', async () => { - const data: Mail = { mail: { plainTextContent: 'content', subject: 'Test' }, recipients: ['test@example.com'] }; - const amqpConnectionSpy = jest.spyOn(amqpConnection, 'publish'); + describe('send', () => { + describe('when recipients array is empty', () => { + it('should not send email', async () => { + const data: Mail = { + mail: { plainTextContent: 'content', subject: 'Test' }, + recipients: ['test@schul-cloud.org'], + }; - await service.send(data); + const amqpConnectionSpy = jest.spyOn(amqpConnection, 'publish'); - const expectedParams = [mailServiceOptions.exchange, mailServiceOptions.routingKey, data, { persistent: true }]; - expect(amqpConnectionSpy).toHaveBeenCalledWith(...expectedParams); + await service.send(data); + + expect(amqpConnectionSpy).toHaveBeenCalledTimes(0); + }); + }); + describe('when sending email', () => { + it('should remove email address that have blacklisted domain and send given data to queue', async () => { + const data: Mail = { + mail: { plainTextContent: 'content', subject: 'Test' }, + recipients: ['test@schul-cloud.org', 'test@example1.com', 'test2@schul-cloud.org', 'test3@schul-cloud.org'], + cc: ['test@example.com', 'test5@schul-cloud.org', 'test6@schul-cloud.org'], + bcc: ['test7@schul-cloud.org', 'test@example2.com', 'test8@schul-cloud.org'], + replyTo: ['test@example3.com', 'test9@schul-cloud.org', 'test10@schul-cloud.org'], + }; + + const amqpConnectionSpy = jest.spyOn(amqpConnection, 'publish'); + + await service.send(data); + + expect(data.recipients).toEqual(['test@example1.com']); + expect(data.cc).toEqual([]); + expect(data.bcc).toEqual(['test@example2.com']); + expect(data.replyTo).toEqual(['test@example3.com']); + + const expectedParams = [mailServiceOptions.exchange, mailServiceOptions.routingKey, data, { persistent: true }]; + expect(amqpConnectionSpy).toHaveBeenCalledWith(...expectedParams); + }); + }); }); }); diff --git a/apps/server/src/infra/mail/mail.service.ts b/apps/server/src/infra/mail/mail.service.ts index aaf9cfacb9d..432f0746934 100644 --- a/apps/server/src/infra/mail/mail.service.ts +++ b/apps/server/src/infra/mail/mail.service.ts @@ -1,7 +1,8 @@ import { AmqpConnection } from '@golevelup/nestjs-rabbitmq'; import { Inject, Injectable } from '@nestjs/common'; - +import { ConfigService } from '@nestjs/config'; import { Mail } from './mail.interface'; +import { IMailConfig } from './interfaces/mail-config'; interface MailServiceOptions { exchange: string; @@ -10,12 +11,47 @@ interface MailServiceOptions { @Injectable() export class MailService { + private readonly domainBlacklist: string[]; + constructor( private readonly amqpConnection: AmqpConnection, - @Inject('MAIL_SERVICE_OPTIONS') private readonly options: MailServiceOptions - ) {} + @Inject('MAIL_SERVICE_OPTIONS') private readonly options: MailServiceOptions, + private readonly configService: ConfigService + ) { + this.domainBlacklist = this.configService.get('ADDITIONAL_BLACKLISTED_EMAIL_DOMAINS'); + } public async send(data: Mail): Promise { + if (this.domainBlacklist.length > 0) { + data.recipients = this.filterEmailAdresses(data.recipients) as string[]; + data.cc = this.filterEmailAdresses(data.cc); + data.bcc = this.filterEmailAdresses(data.bcc); + data.replyTo = this.filterEmailAdresses(data.replyTo); + } + + if (data.recipients.length === 0) { + return; + } + await this.amqpConnection.publish(this.options.exchange, this.options.routingKey, data, { persistent: true }); } + + private filterEmailAdresses(mails: string[] | undefined): string[] | undefined { + if (mails === undefined || mails === null) { + return mails; + } + const mailWhitelist: string[] = []; + + for (const mail of mails) { + const mailDomain = this.getMailDomain(mail); + if (mailDomain && !this.domainBlacklist.includes(mailDomain)) { + mailWhitelist.push(mail); + } + } + return mailWhitelist; + } + + private getMailDomain(mail: string): string { + return mail.split('@')[1]; + } } diff --git a/apps/server/src/modules/server/server.config.ts b/apps/server/src/modules/server/server.config.ts index becc7ec78fc..5d1ea95cc3b 100644 --- a/apps/server/src/modules/server/server.config.ts +++ b/apps/server/src/modules/server/server.config.ts @@ -5,6 +5,7 @@ import type { IAccountConfig } from '@modules/account'; import type { IFilesStorageClientConfig } from '@modules/files-storage-client'; import type { IUserConfig } from '@modules/user'; import type { ICommonCartridgeConfig } from '@modules/learnroom/common-cartridge'; +import { IMailConfig } from '@src/infra/mail/interfaces/mail-config'; export enum NodeEnvType { TEST = 'test', @@ -19,7 +20,8 @@ export interface IServerConfig IFilesStorageClientConfig, IAccountConfig, IIdentityManagementConfig, - ICommonCartridgeConfig { + ICommonCartridgeConfig, + IMailConfig { NODE_ENV: string; SC_DOMAIN: string; } @@ -39,6 +41,9 @@ const config: IServerConfig = { FEATURE_IDENTITY_MANAGEMENT_ENABLED: Configuration.get('FEATURE_IDENTITY_MANAGEMENT_ENABLED') as boolean, FEATURE_IDENTITY_MANAGEMENT_STORE_ENABLED: Configuration.get('FEATURE_IDENTITY_MANAGEMENT_STORE_ENABLED') as boolean, FEATURE_IDENTITY_MANAGEMENT_LOGIN_ENABLED: Configuration.get('FEATURE_IDENTITY_MANAGEMENT_LOGIN_ENABLED') as boolean, + ADDITIONAL_BLACKLISTED_EMAIL_DOMAINS: (Configuration.get('ADDITIONAL_BLACKLISTED_EMAIL_DOMAINS') as string) + .split(',') + .map((domain) => domain.trim()), }; export const serverConfig = () => config; diff --git a/config/default.schema.json b/config/default.schema.json index a34d8e899ad..bd637b719e9 100644 --- a/config/default.schema.json +++ b/config/default.schema.json @@ -177,6 +177,7 @@ }, "ADDITIONAL_BLACKLISTED_EMAIL_DOMAINS": { "type": "string", + "default":"", "description": "Add custom domain to the list of blocked domains (comma separated list)." }, "FEATURE_TSP_AUTO_CONSENT_ENABLED": {