Skip to content

Commit

Permalink
feat(core): Implement permitted mime types for Assets
Browse files Browse the repository at this point in the history
Relates to #421
  • Loading branch information
michaelbromley committed Aug 18, 2020
1 parent 75f1ff5 commit 272b2db
Show file tree
Hide file tree
Showing 8 changed files with 138 additions and 40 deletions.
119 changes: 84 additions & 35 deletions packages/core/e2e/asset.e2e-spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
/* tslint:disable:no-non-null-assertion */
import { mergeConfig } from '@vendure/common/lib/merge-config';
import { omit } from '@vendure/common/lib/omit';
import { createTestEnvironment } from '@vendure/testing';
import gql from 'graphql-tag';
import path from 'path';

import { initialData } from '../../../e2e-common/e2e-initial-data';
import { TEST_SETUP_TIMEOUT_MS, testConfig } from '../../../e2e-common/test-config';
import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';

import { ASSET_FRAGMENT } from './graphql/fragments';
import {
Expand All @@ -24,9 +25,16 @@ import {
GET_PRODUCT_WITH_VARIANTS,
UPDATE_ASSET,
} from './graphql/shared-definitions';
import { assertThrowsWithMessage } from './utils/assert-throws-with-message';

describe('Asset resolver', () => {
const { server, adminClient } = createTestEnvironment(testConfig);
const { server, adminClient } = createTestEnvironment(
mergeConfig(testConfig, {
assetOptions: {
permittedFileTypes: ['image/*', '.pdf'],
},
}),
);

let firstAssetId: string;
let createdAssetId: string;
Expand Down Expand Up @@ -57,7 +65,7 @@ describe('Asset resolver', () => {
);

expect(assets.totalItems).toBe(4);
expect(assets.items.map((a) => omit(a, ['id']))).toEqual([
expect(assets.items.map(a => omit(a, ['id']))).toEqual([
{
fileSize: 1680,
mimeType: 'image/jpeg',
Expand Down Expand Up @@ -113,41 +121,82 @@ describe('Asset resolver', () => {
});
});

it('createAssets', async () => {
const filesToUpload = [
path.join(__dirname, 'fixtures/assets/pps1.jpg'),
path.join(__dirname, 'fixtures/assets/pps2.jpg'),
];
const { createAssets }: CreateAssets.Mutation = await adminClient.fileUploadMutation({
mutation: CREATE_ASSETS,
filePaths: filesToUpload,
mapVariables: (filePaths) => ({
input: filePaths.map((p) => ({ file: null })),
}),
describe('createAssets', () => {
it('permitted types by mime type', async () => {
const filesToUpload = [
path.join(__dirname, 'fixtures/assets/pps1.jpg'),
path.join(__dirname, 'fixtures/assets/pps2.jpg'),
];
const { createAssets }: CreateAssets.Mutation = await adminClient.fileUploadMutation({
mutation: CREATE_ASSETS,
filePaths: filesToUpload,
mapVariables: filePaths => ({
input: filePaths.map(p => ({ file: null })),
}),
});

expect(createAssets.map(a => omit(a, ['id'])).sort((a, b) => (a.name < b.name ? -1 : 1))).toEqual(
[
{
fileSize: 1680,
focalPoint: null,
mimeType: 'image/jpeg',
name: 'pps1.jpg',
preview: 'test-url/test-assets/pps1__preview.jpg',
source: 'test-url/test-assets/pps1.jpg',
type: 'IMAGE',
},
{
fileSize: 1680,
focalPoint: null,
mimeType: 'image/jpeg',
name: 'pps2.jpg',
preview: 'test-url/test-assets/pps2__preview.jpg',
source: 'test-url/test-assets/pps2.jpg',
type: 'IMAGE',
},
],
);

createdAssetId = createAssets[0].id;
});

expect(createAssets.map((a) => omit(a, ['id'])).sort((a, b) => (a.name < b.name ? -1 : 1))).toEqual([
{
fileSize: 1680,
focalPoint: null,
mimeType: 'image/jpeg',
name: 'pps1.jpg',
preview: 'test-url/test-assets/pps1__preview.jpg',
source: 'test-url/test-assets/pps1.jpg',
type: 'IMAGE',
},
{
fileSize: 1680,
focalPoint: null,
mimeType: 'image/jpeg',
name: 'pps2.jpg',
preview: 'test-url/test-assets/pps2__preview.jpg',
source: 'test-url/test-assets/pps2.jpg',
type: 'IMAGE',
},
]);
it('permitted type by file extension', async () => {
const filesToUpload = [path.join(__dirname, 'fixtures/assets/dummy.pdf')];
const { createAssets }: CreateAssets.Mutation = await adminClient.fileUploadMutation({
mutation: CREATE_ASSETS,
filePaths: filesToUpload,
mapVariables: filePaths => ({
input: filePaths.map(p => ({ file: null })),
}),
});

expect(createAssets.map(a => omit(a, ['id']))).toEqual([
{
fileSize: 1680,
focalPoint: null,
mimeType: 'application/pdf',
name: 'dummy.pdf',
preview: 'test-url/test-assets/dummy__preview.pdf.png',
source: 'test-url/test-assets/dummy.pdf',
type: 'BINARY',
},
]);
});

createdAssetId = createAssets[0].id;
it(
'not permitted type',
assertThrowsWithMessage(async () => {
const filesToUpload = [path.join(__dirname, 'fixtures/assets/dummy.txt')];
const { createAssets }: CreateAssets.Mutation = await adminClient.fileUploadMutation({
mutation: CREATE_ASSETS,
filePaths: filesToUpload,
mapVariables: filePaths => ({
input: filePaths.map(p => ({ file: null })),
}),
});
}, `The MIME type 'text/plain' is not permitted.`),
);
});

describe('updateAsset', () => {
Expand Down
Binary file added packages/core/e2e/fixtures/assets/dummy.pdf
Binary file not shown.
1 change: 1 addition & 0 deletions packages/core/e2e/fixtures/assets/dummy.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
hi!
5 changes: 5 additions & 0 deletions packages/core/src/api/resolvers/admin/asset.resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ export class AssetResolver {
@Mutation()
@Allow(Permission.CreateCatalog)
async createAssets(@Ctx() ctx: RequestContext, @Args() args: MutationCreateAssetsArgs): Promise<Asset[]> {
// TODO: Currently we validate _all_ mime types up-front due to limitations
// with the existing error handling mechanisms. With a solution as described
// in https://github.com/vendure-ecommerce/vendure/issues/437 we could defer
// this check to the individual processing of a single Asset.
await this.assetService.validateInputMimeTypes(args.input);
// TODO: Is there some way to parellelize this while still preserving
// the order of files in the upload? Non-deterministic IDs mess up the e2e test snapshots.
const assets: Asset[] = [];
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/config/default-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ export const defaultConfig: RuntimeVendureConfig = {
assetNamingStrategy: new DefaultAssetNamingStrategy(),
assetStorageStrategy: new NoAssetStorageStrategy(),
assetPreviewStrategy: new NoAssetPreviewStrategy(),
permittedFileTypes: ['image/*', 'video/*', 'audio/*', '.pdf'],
uploadMaxFileSize: 20971520,
},
dbConnectionOptions: {
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/config/vendure-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,16 @@ export interface AssetOptions {
* @default NoAssetPreviewStrategy
*/
assetPreviewStrategy: AssetPreviewStrategy;
/**
* @description
* An array of the permitted file types that may be uploaded as Assets. Each entry
* should be in the form of a valid
* [unique file type specifier](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file#Unique_file_type_specifiers)
* i.e. either a file extension (".pdf") or a mime type ("image/*", "audio/mpeg" etc.).
*
* @default image, audio, video MIME types plus PDFs
*/
permittedFileTypes: string[];
/**
* @description
* The max file size in bytes for uploaded assets.
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/i18n/messages/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"identifier-change-token-has-expired": "Identifier change token has expired",
"invalid-sort-field": "The sort field '{ fieldName }' is invalid. Valid fields are: { validFields }",
"language-not-available-in-global-settings": "Language \"{code}\" is not available. First enable it via GlobalSettings and try again.",
"mime-type-not-permitted": "The MIME type '{ mimetype }' is not permitted.",
"missing-password-on-registration": "A password must be provided when `authOptions.requireVerification` is set to \"false\"",
"no-active-tax-zone": "The active tax zone could not be determined. Ensure a default tax zone is set for the current channel.",
"no-search-plugin-configured": "No search plugin has been configured",
Expand Down
41 changes: 36 additions & 5 deletions packages/core/src/service/services/asset.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { Stream } from 'stream';
import { Connection, Like } from 'typeorm';

import { RequestContext } from '../../api/common/request-context';
import { InternalServerError } from '../../common/error/errors';
import { InternalServerError, UserInputError } from '../../common/error/errors';
import { ListQueryOptions } from '../../common/types/common-types';
import { getAssetType, idsAreEqual } from '../../common/utils';
import { ConfigService } from '../../config/config.service';
Expand Down Expand Up @@ -47,12 +47,22 @@ export interface EntityAssetInput {

@Injectable()
export class AssetService {
private permittedMimeTypes: Array<{ type: string; subtype: string }> = [];

constructor(
@InjectConnection() private connection: Connection,
private configService: ConfigService,
private listQueryBuilder: ListQueryBuilder,
private eventBus: EventBus,
) {}
) {
this.permittedMimeTypes = this.configService.assetOptions.permittedFileTypes
.map(val => (/\.[\w]+/.test(val) ? mime.lookup(val) || undefined : val))
.filter(notNullOrUndefined)
.map(val => {
const [type, subtype] = val.split('/');
return { type, subtype };
});
}

findOne(id: ID): Promise<Asset | undefined> {
return this.connection.getRepository(Asset).findOne(id);
Expand Down Expand Up @@ -91,7 +101,7 @@ export class AssetService {
});
assets = (entityWithAssets && entityWithAssets.assets) || [];
}
return assets.sort((a, b) => a.position - b.position).map((a) => a.asset);
return assets.sort((a, b) => a.position - b.position).map(a => a.asset);
}

async updateFeaturedAsset<T extends EntityWithAssets>(entity: T, input: EntityAssetInput): Promise<T> {
Expand Down Expand Up @@ -121,7 +131,7 @@ export class AssetService {
if (assetIds && assetIds.length) {
const assets = await this.connection.getRepository(Asset).findByIds(assetIds);
const sortedAssets = assetIds
.map((id) => assets.find((a) => idsAreEqual(a.id, id)))
.map(id => assets.find(a => idsAreEqual(a.id, id)))
.filter(notNullOrUndefined);
await this.removeExistingOrderableAssets(entity);
entity.assets = await this.createOrderableAssets(entity, sortedAssets);
Expand All @@ -131,6 +141,15 @@ export class AssetService {
return entity;
}

async validateInputMimeTypes(inputs: CreateAssetInput[]): Promise<void> {
for (const input of inputs) {
const { mimetype } = await input.file;
if (!this.validateMimeType(mimetype)) {
throw new UserInputError('error.mime-type-not-permitted', { mimetype });
}
}
}

/**
* Create an Asset based on a file uploaded via the GraphQL API.
*/
Expand Down Expand Up @@ -214,6 +233,9 @@ export class AssetService {

private async createAssetInternal(stream: Stream, filename: string, mimetype: string): Promise<Asset> {
const { assetOptions } = this.configService;
if (!this.validateMimeType(mimetype)) {
throw new UserInputError('error.mime-type-not-permitted', { mimetype });
}
const { assetPreviewStrategy, assetStorageStrategy } = assetOptions;
const sourceFileName = await this.getSourceFileName(filename);
const previewFileName = await this.getPreviewFileName(sourceFileName);
Expand Down Expand Up @@ -310,7 +332,7 @@ export class AssetService {
private getOrderableAssetType(entity: EntityWithAssets): Type<OrderableAsset> {
const assetRelation = this.connection
.getRepository(entity.constructor)
.metadata.relations.find((r) => r.propertyName === 'assets');
.metadata.relations.find(r => r.propertyName === 'assets');
if (!assetRelation || typeof assetRelation.type === 'string') {
throw new InternalServerError('error.could-not-find-matching-orderable-asset');
}
Expand All @@ -331,6 +353,15 @@ export class AssetService {
}
}

private validateMimeType(mimeType: string): boolean {
const [type, subtype] = mimeType.split('/');
const typeMatch = this.permittedMimeTypes.find(t => t.type === type);
if (typeMatch) {
return typeMatch.subtype === subtype || typeMatch.subtype === '*';
}
return false;
}

/**
* Find the entities which reference the given Asset as a featuredAsset.
*/
Expand Down

0 comments on commit 272b2db

Please sign in to comment.