Skip to content

Commit

Permalink
Merge branch 'main' into N21-1029-renaming-of-collections
Browse files Browse the repository at this point in the history
  • Loading branch information
@arnegns
arnegns committed Nov 9, 2023
2 parents e51519d + f81f023 commit 8dc1faa
Show file tree
Hide file tree
Showing 8 changed files with 190 additions and 40 deletions.
3 changes: 3 additions & 0 deletions apps/server/src/infra/mail/interfaces/mail-config.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export interface IMailConfig {
ADDITIONAL_BLACKLISTED_EMAIL_DOMAINS: string[];
}
3 changes: 3 additions & 0 deletions apps/server/src/infra/mail/mail.module.ts
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -17,6 +19,7 @@ export class MailModule {
provide: 'MAIL_SERVICE_OPTIONS',
useValue: { exchange: options.exchange, routingKey: options.routingKey },
},
ConfigService<IMailConfig, true>,
],
exports: [MailService],
};
Expand Down
49 changes: 43 additions & 6 deletions apps/server/src/infra/mail/mail.service.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -19,6 +22,10 @@ describe('MailService', () => {
MailService,
{ provide: AmqpConnection, useValue: { publish: () => {} } },
{ provide: 'MAIL_SERVICE_OPTIONS', useValue: mailServiceOptions },
{
provide: ConfigService,
useValue: createMock<ConfigService<IMailConfig, true>>({ get: () => ['schul-cloud.org', 'example.com'] }),
},
],
}).compile();

Expand All @@ -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: ['[email protected]'] };
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: ['[email protected]'],
};

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: ['[email protected]', '[email protected]', '[email protected]', '[email protected]'],
cc: ['[email protected]', '[email protected]', '[email protected]'],
bcc: ['[email protected]', '[email protected]', '[email protected]'],
replyTo: ['[email protected]', '[email protected]', '[email protected]'],
};

const amqpConnectionSpy = jest.spyOn(amqpConnection, 'publish');

await service.send(data);

expect(data.recipients).toEqual(['[email protected]']);
expect(data.cc).toEqual([]);
expect(data.bcc).toEqual(['[email protected]']);
expect(data.replyTo).toEqual(['[email protected]']);

const expectedParams = [mailServiceOptions.exchange, mailServiceOptions.routingKey, data, { persistent: true }];
expect(amqpConnectionSpy).toHaveBeenCalledWith(...expectedParams);
});
});
});
});
42 changes: 39 additions & 3 deletions apps/server/src/infra/mail/mail.service.ts
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<IMailConfig, true>
) {
this.domainBlacklist = this.configService.get<string[]>('ADDITIONAL_BLACKLISTED_EMAIL_DOMAINS');
}

public async send(data: Mail): Promise<void> {
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];
}
}
111 changes: 83 additions & 28 deletions apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { AntivirusService } from '@infra/antivirus';
import { PreviewProducer } from '@infra/preview-generator';
import { S3ClientAdapter } from '@infra/s3-client';
import { EntityManager, ObjectId } from '@mikro-orm/mongodb';
import { ICurrentUser } from '@modules/authentication';
import { JwtAuthGuard } from '@modules/authentication/guard/jwt-auth.guard';
import { ExecutionContext, INestApplication, NotFoundException, StreamableFile } from '@nestjs/common';
import { Test, TestingModule } from '@nestjs/testing';
import { ApiValidationError } from '@shared/common';
import { EntityId, Permission } from '@shared/domain';
import { AntivirusService } from '@infra/antivirus';
import { PreviewProducer } from '@infra/preview-generator';
import { S3ClientAdapter } from '@infra/s3-client';
import { cleanupCollections, mapUserToCurrentUser, roleFactory, schoolFactory, userFactory } from '@shared/testing';
import NodeClam from 'clamscan';
import { Request } from 'express';
Expand Down Expand Up @@ -63,6 +63,19 @@ class API {
};
}

async getPreviewWithEtag(routeName: string, etag: string, query?: string | Record<string, unknown>) {
const response = await request(this.app.getHttpServer())
.get(routeName)
.query(query || {})
.set('If-None-Match', etag);

return {
result: response.body as StreamableFile,
error: response.body as ApiValidationError,
status: response.status,
};
}

async getPreviewBytesRange(routeName: string, bytesRange: string, query?: string | Record<string, unknown>) {
const response = await request(this.app.getHttpServer())
.get(routeName)
Expand Down Expand Up @@ -299,34 +312,75 @@ describe('File Controller (API) - preview', () => {
return { uploadedFile };
};

it('should return status 200 for successful download', async () => {
const { uploadedFile } = await setup();
const query = {
...defaultQueryParameters,
forceUpdate: false,
};

const response = await api.getPreview(`/file/preview/${uploadedFile.id}/${uploadedFile.name}`, query);

expect(response.status).toEqual(200);
describe('WHEN header contains no etag', () => {
it('should return status 200 for successful download', async () => {
const { uploadedFile } = await setup();
const query = {
...defaultQueryParameters,
forceUpdate: false,
};

const response = await api.getPreview(`/file/preview/${uploadedFile.id}/${uploadedFile.name}`, query);

expect(response.status).toEqual(200);
});

it('should return status 206 and required headers for the successful partial file stream download', async () => {
const { uploadedFile } = await setup();
const query = {
...defaultQueryParameters,
forceUpdate: false,
};

const response = await api.getPreviewBytesRange(
`/file/preview/${uploadedFile.id}/${uploadedFile.name}`,
'bytes=0-',
query
);

expect(response.status).toEqual(206);
expect(response.headers['accept-ranges']).toMatch('bytes');
expect(response.headers['content-range']).toMatch('bytes 0-3/4');
expect(response.headers.etag).toMatch('testTag');
});
});

it('should return status 206 and required headers for the successful partial file stream download', async () => {
const { uploadedFile } = await setup();
const query = {
...defaultQueryParameters,
forceUpdate: false,
};

const response = await api.getPreviewBytesRange(
`/file/preview/${uploadedFile.id}/${uploadedFile.name}`,
'bytes=0-',
query
);
describe('WHEN header contains not matching etag', () => {
it('should return status 200 for successful download', async () => {
const { uploadedFile } = await setup();
const query = {
...defaultQueryParameters,
forceUpdate: false,
};
const etag = 'otherTag';

const response = await api.getPreviewWithEtag(
`/file/preview/${uploadedFile.id}/${uploadedFile.name}`,
etag,
query
);

expect(response.status).toEqual(200);
});
});

expect(response.status).toEqual(206);
expect(response.headers['accept-ranges']).toMatch('bytes');
expect(response.headers['content-range']).toMatch('bytes 0-3/4');
describe('WHEN header contains matching etag', () => {
it('should return status 304', async () => {
const { uploadedFile } = await setup();
const query = {
...defaultQueryParameters,
forceUpdate: false,
};
const etag = 'testTag';

const response = await api.getPreviewWithEtag(
`/file/preview/${uploadedFile.id}/${uploadedFile.name}`,
etag,
query
);

expect(response.status).toEqual(304);
});
});
});

Expand Down Expand Up @@ -369,6 +423,7 @@ describe('File Controller (API) - preview', () => {
expect(response.status).toEqual(206);
expect(response.headers['accept-ranges']).toMatch('bytes');
expect(response.headers['content-range']).toMatch('bytes 0-3/4');
expect(response.headers.etag).toMatch('testTag');
});
});
});
Expand Down
14 changes: 12 additions & 2 deletions apps/server/src/modules/files-storage/controller/files-storage.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ export class FilesStorageController {
@ApiOperation({ summary: 'Streamable download of a preview file.' })
@ApiResponse({ status: 200, type: StreamableFile })
@ApiResponse({ status: 206, type: StreamableFile })
@ApiResponse({ status: 304, description: 'Not Modified' })
@ApiResponse({ status: 400, type: ApiValidationError })
@ApiResponse({ status: 403, type: ForbiddenException })
@ApiResponse({ status: 404, type: NotFoundException })
Expand All @@ -134,15 +135,24 @@ export class FilesStorageController {
@Query() previewParams: PreviewParams,
@Req() req: Request,
@Res({ passthrough: true }) response: Response,
@Headers('Range') bytesRange?: string
): Promise<StreamableFile> {
@Headers('Range') bytesRange?: string,
@Headers('If-None-Match') etag?: string
): Promise<StreamableFile | void> {
const fileResponse = await this.filesStorageUC.downloadPreview(
currentUser.userId,
params,
previewParams,
bytesRange
);

response.set({ ETag: fileResponse.etag });

if (etag === fileResponse.etag) {
response.status(HttpStatus.NOT_MODIFIED);

return undefined;
}

const streamableFile = this.streamFileToClient(req, fileResponse, response, bytesRange);

return streamableFile;
Expand Down
7 changes: 6 additions & 1 deletion apps/server/src/modules/server/server.config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -19,7 +20,8 @@ export interface IServerConfig
IFilesStorageClientConfig,
IAccountConfig,
IIdentityManagementConfig,
ICommonCartridgeConfig {
ICommonCartridgeConfig,
IMailConfig {
NODE_ENV: string;
SC_DOMAIN: string;
}
Expand All @@ -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;
1 change: 1 addition & 0 deletions config/default.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down

0 comments on commit 8dc1faa

Please sign in to comment.