Skip to content

Commit f81f023

Browse files
authored
BC-4895-stop sending emails to non existent thr @schul-cloud.org addresses (#4476)
* impl of mail addresses whitelist logic
1 parent a7273a1 commit f81f023

File tree

6 files changed

+95
-10
lines changed

6 files changed

+95
-10
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export interface IMailConfig {
2+
ADDITIONAL_BLACKLISTED_EMAIL_DOMAINS: string[];
3+
}

apps/server/src/infra/mail/mail.module.ts

+3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { Module, DynamicModule } from '@nestjs/common';
2+
import { ConfigService } from '@nestjs/config';
23
import { MailService } from './mail.service';
4+
import { IMailConfig } from './interfaces/mail-config';
35

46
interface MailModuleOptions {
57
exchange: string;
@@ -17,6 +19,7 @@ export class MailModule {
1719
provide: 'MAIL_SERVICE_OPTIONS',
1820
useValue: { exchange: options.exchange, routingKey: options.routingKey },
1921
},
22+
ConfigService<IMailConfig, true>,
2023
],
2124
exports: [MailService],
2225
};

apps/server/src/infra/mail/mail.service.spec.ts

+43-6
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { AmqpConnection } from '@golevelup/nestjs-rabbitmq';
22
import { Test, TestingModule } from '@nestjs/testing';
3+
import { createMock } from '@golevelup/ts-jest';
4+
import { ConfigService } from '@nestjs/config';
35
import { Mail } from './mail.interface';
46
import { MailService } from './mail.service';
7+
import { IMailConfig } from './interfaces/mail-config';
58

69
describe('MailService', () => {
710
let module: TestingModule;
@@ -19,6 +22,10 @@ describe('MailService', () => {
1922
MailService,
2023
{ provide: AmqpConnection, useValue: { publish: () => {} } },
2124
{ provide: 'MAIL_SERVICE_OPTIONS', useValue: mailServiceOptions },
25+
{
26+
provide: ConfigService,
27+
useValue: createMock<ConfigService<IMailConfig, true>>({ get: () => ['schul-cloud.org', 'example.com'] }),
28+
},
2229
],
2330
}).compile();
2431

@@ -34,13 +41,43 @@ describe('MailService', () => {
3441
expect(service).toBeDefined();
3542
});
3643

37-
it('should send given data to queue', async () => {
38-
const data: Mail = { mail: { plainTextContent: 'content', subject: 'Test' }, recipients: ['[email protected]'] };
39-
const amqpConnectionSpy = jest.spyOn(amqpConnection, 'publish');
44+
describe('send', () => {
45+
describe('when recipients array is empty', () => {
46+
it('should not send email', async () => {
47+
const data: Mail = {
48+
mail: { plainTextContent: 'content', subject: 'Test' },
49+
recipients: ['[email protected]'],
50+
};
4051

41-
await service.send(data);
52+
const amqpConnectionSpy = jest.spyOn(amqpConnection, 'publish');
4253

43-
const expectedParams = [mailServiceOptions.exchange, mailServiceOptions.routingKey, data, { persistent: true }];
44-
expect(amqpConnectionSpy).toHaveBeenCalledWith(...expectedParams);
54+
await service.send(data);
55+
56+
expect(amqpConnectionSpy).toHaveBeenCalledTimes(0);
57+
});
58+
});
59+
describe('when sending email', () => {
60+
it('should remove email address that have blacklisted domain and send given data to queue', async () => {
61+
const data: Mail = {
62+
mail: { plainTextContent: 'content', subject: 'Test' },
63+
64+
65+
66+
67+
};
68+
69+
const amqpConnectionSpy = jest.spyOn(amqpConnection, 'publish');
70+
71+
await service.send(data);
72+
73+
expect(data.recipients).toEqual(['[email protected]']);
74+
expect(data.cc).toEqual([]);
75+
expect(data.bcc).toEqual(['[email protected]']);
76+
expect(data.replyTo).toEqual(['[email protected]']);
77+
78+
const expectedParams = [mailServiceOptions.exchange, mailServiceOptions.routingKey, data, { persistent: true }];
79+
expect(amqpConnectionSpy).toHaveBeenCalledWith(...expectedParams);
80+
});
81+
});
4582
});
4683
});
+39-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { AmqpConnection } from '@golevelup/nestjs-rabbitmq';
22
import { Inject, Injectable } from '@nestjs/common';
3-
3+
import { ConfigService } from '@nestjs/config';
44
import { Mail } from './mail.interface';
5+
import { IMailConfig } from './interfaces/mail-config';
56

67
interface MailServiceOptions {
78
exchange: string;
@@ -10,12 +11,47 @@ interface MailServiceOptions {
1011

1112
@Injectable()
1213
export class MailService {
14+
private readonly domainBlacklist: string[];
15+
1316
constructor(
1417
private readonly amqpConnection: AmqpConnection,
15-
@Inject('MAIL_SERVICE_OPTIONS') private readonly options: MailServiceOptions
16-
) {}
18+
@Inject('MAIL_SERVICE_OPTIONS') private readonly options: MailServiceOptions,
19+
private readonly configService: ConfigService<IMailConfig, true>
20+
) {
21+
this.domainBlacklist = this.configService.get<string[]>('ADDITIONAL_BLACKLISTED_EMAIL_DOMAINS');
22+
}
1723

1824
public async send(data: Mail): Promise<void> {
25+
if (this.domainBlacklist.length > 0) {
26+
data.recipients = this.filterEmailAdresses(data.recipients) as string[];
27+
data.cc = this.filterEmailAdresses(data.cc);
28+
data.bcc = this.filterEmailAdresses(data.bcc);
29+
data.replyTo = this.filterEmailAdresses(data.replyTo);
30+
}
31+
32+
if (data.recipients.length === 0) {
33+
return;
34+
}
35+
1936
await this.amqpConnection.publish(this.options.exchange, this.options.routingKey, data, { persistent: true });
2037
}
38+
39+
private filterEmailAdresses(mails: string[] | undefined): string[] | undefined {
40+
if (mails === undefined || mails === null) {
41+
return mails;
42+
}
43+
const mailWhitelist: string[] = [];
44+
45+
for (const mail of mails) {
46+
const mailDomain = this.getMailDomain(mail);
47+
if (mailDomain && !this.domainBlacklist.includes(mailDomain)) {
48+
mailWhitelist.push(mail);
49+
}
50+
}
51+
return mailWhitelist;
52+
}
53+
54+
private getMailDomain(mail: string): string {
55+
return mail.split('@')[1];
56+
}
2157
}

apps/server/src/modules/server/server.config.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { IAccountConfig } from '@modules/account';
55
import type { IFilesStorageClientConfig } from '@modules/files-storage-client';
66
import type { IUserConfig } from '@modules/user';
77
import type { ICommonCartridgeConfig } from '@modules/learnroom/common-cartridge';
8+
import { IMailConfig } from '@src/infra/mail/interfaces/mail-config';
89

910
export enum NodeEnvType {
1011
TEST = 'test',
@@ -19,7 +20,8 @@ export interface IServerConfig
1920
IFilesStorageClientConfig,
2021
IAccountConfig,
2122
IIdentityManagementConfig,
22-
ICommonCartridgeConfig {
23+
ICommonCartridgeConfig,
24+
IMailConfig {
2325
NODE_ENV: string;
2426
SC_DOMAIN: string;
2527
}
@@ -39,6 +41,9 @@ const config: IServerConfig = {
3941
FEATURE_IDENTITY_MANAGEMENT_ENABLED: Configuration.get('FEATURE_IDENTITY_MANAGEMENT_ENABLED') as boolean,
4042
FEATURE_IDENTITY_MANAGEMENT_STORE_ENABLED: Configuration.get('FEATURE_IDENTITY_MANAGEMENT_STORE_ENABLED') as boolean,
4143
FEATURE_IDENTITY_MANAGEMENT_LOGIN_ENABLED: Configuration.get('FEATURE_IDENTITY_MANAGEMENT_LOGIN_ENABLED') as boolean,
44+
ADDITIONAL_BLACKLISTED_EMAIL_DOMAINS: (Configuration.get('ADDITIONAL_BLACKLISTED_EMAIL_DOMAINS') as string)
45+
.split(',')
46+
.map((domain) => domain.trim()),
4247
};
4348

4449
export const serverConfig = () => config;

config/default.schema.json

+1
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@
177177
},
178178
"ADDITIONAL_BLACKLISTED_EMAIL_DOMAINS": {
179179
"type": "string",
180+
"default":"",
180181
"description": "Add custom domain to the list of blocked domains (comma separated list)."
181182
},
182183
"FEATURE_TSP_AUTO_CONSENT_ENABLED": {

0 commit comments

Comments
 (0)