Skip to content

Commit

Permalink
fix: Accepted Media Types settings validation (#32478)
Browse files Browse the repository at this point in the history
Co-authored-by: Henrique Guimarães Ribeiro <43561537+rique223@users.noreply.github.com>
  • Loading branch information
hugocostadev and rique223 authored Jun 14, 2024
1 parent afa560d commit 97eaa17
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 18 deletions.
5 changes: 5 additions & 0 deletions .changeset/chilly-toys-hunt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@rocket.chat/meteor": patch
---

Fixed "File Upload > Accepted Media Types" setting to allow all type of files uploads
3 changes: 1 addition & 2 deletions apps/meteor/app/file-upload/server/lib/FileUpload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export const FileUpload = {
throw new Meteor.Error('error-file-too-large', reason);
}

if (!fileUploadIsValidContentType(file.type as string, '')) {
if (!fileUploadIsValidContentType(file?.type)) {
const reason = i18n.t('File_type_is_not_accepted', { lng: language });
throw new Meteor.Error('error-invalid-file-type', reason);
}
Expand Down Expand Up @@ -420,7 +420,6 @@ export const FileUpload = {
await Avatars.deleteFile(oldAvatar._id);
}
await Avatars.updateFileNameById(file._id, user.username);
// console.log('upload finished ->', file);
},

async requestCanAccessFiles({ headers = {}, url }: http.IncomingMessage, file?: IUpload) {
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/app/utils/client/restrictions.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { settings } from '../../settings/client';
import { fileUploadIsValidContentTypeFromSettings } from '../lib/restrictions';

export const fileUploadIsValidContentType = function (type: string, customWhiteList?: string): boolean {
export const fileUploadIsValidContentType = function (type: string | undefined, customWhiteList?: string): boolean {
const blackList = settings.get<string>('FileUpload_MediaTypeBlackList');
const whiteList = customWhiteList || settings.get<string>('FileUpload_MediaTypeWhiteList');

Expand Down
34 changes: 21 additions & 13 deletions apps/meteor/app/utils/lib/restrictions.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import _ from 'underscore';

export const fileUploadMediaWhiteList = function (customWhiteList: string): string[] | undefined {
const mediaTypeWhiteList = customWhiteList;

if (!mediaTypeWhiteList || mediaTypeWhiteList === '*') {
return;
}
return _.map(mediaTypeWhiteList.split(','), (item) => {
return mediaTypeWhiteList.split(',').map((item) => {
return item.trim();
});
};
Expand All @@ -17,37 +15,47 @@ const fileUploadMediaBlackList = function (customBlackList: string): string[] |
return;
}

return _.map(blacklist.split(','), (item) => item.trim());
return blacklist.split(',').map((item) => item.trim());
};

const isTypeOnList = function (type: string, list: string[]): boolean | undefined {
if (_.contains(list, type)) {
const isTypeOnList = function (type?: string, list?: string[]): boolean {
if (!type || !list) {
return false;
}

if (list.includes(type)) {
return true;
}
const wildCardGlob = '/*';
const wildcards = _.filter(list, (item) => {
const wildcards = list.filter((item) => {
return item.indexOf(wildCardGlob) > 0;
});
if (_.contains(wildcards, type.replace(/(\/.*)$/, wildCardGlob))) {
if (wildcards.includes(type.replace(/(\/.*)$/, wildCardGlob))) {
return true;
}

return false;
};

export const fileUploadIsValidContentTypeFromSettings = function (type: string, customWhiteList: string, customBlackList: string): boolean {
export const fileUploadIsValidContentTypeFromSettings = function (
type: string | undefined,
customWhiteList: string,
customBlackList: string,
): boolean {
const blackList = fileUploadMediaBlackList(customBlackList);
const whiteList = fileUploadMediaWhiteList(customWhiteList);

if (!type && blackList) {
if (blackList && type && isTypeOnList(type, blackList)) {
return false;
}

if (blackList && isTypeOnList(type, blackList)) {
return false;
if (whiteList) {
return isTypeOnList(type, whiteList);
}

if (!whiteList) {
return true;
}

return !!isTypeOnList(type, whiteList);
return false;
};
2 changes: 1 addition & 1 deletion apps/meteor/app/utils/server/restrictions.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { settings } from '../../settings/server';
import { fileUploadIsValidContentTypeFromSettings } from '../lib/restrictions';

export const fileUploadIsValidContentType = function (type: string, customWhiteList?: string): boolean {
export const fileUploadIsValidContentType = function (type: string | undefined, customWhiteList?: string): boolean {
const blackList = settings.get<string>('FileUpload_MediaTypeBlackList');
const whiteList = customWhiteList || settings.get<string>('FileUpload_MediaTypeWhiteList');

Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/client/lib/chats/flows/uploadFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export const uploadFiles = async (chat: ChatAPI, files: readonly File[], resetFi
imperativeModal.close();
uploadNextFile();
},
invalidContentType: !(file.type && fileUploadIsValidContentType(file.type)),
invalidContentType: !fileUploadIsValidContentType(file?.type),
},
});
};
Expand Down
49 changes: 49 additions & 0 deletions apps/meteor/tests/unit/app/utils/lib/restrictions.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { expect } from 'chai';

import { fileUploadIsValidContentTypeFromSettings } from '../../../../../app/utils/lib/restrictions';

describe('fileUploadIsValidContentTypeFromSettings', () => {
it('should return true if type is not defined and whiteList is not defined', () => {
expect(fileUploadIsValidContentTypeFromSettings(undefined, '', '')).to.be.true;
});

it('should return false if type is not defined and whiteList is defined', () => {
expect(fileUploadIsValidContentTypeFromSettings(undefined, 'image/jpeg', '')).to.be.false;
});

it('should return true if type is defined and whiteList is not defined', () => {
expect(fileUploadIsValidContentTypeFromSettings('image/jpeg', '', '')).to.be.true;
});

it('should return true if type is defined and whiteList is defined and type is in whiteList', () => {
expect(fileUploadIsValidContentTypeFromSettings('image/jpeg', 'image/jpeg', '')).to.be.true;
});

it('should return false if type is defined and whiteList is defined and type is not in whiteList', () => {
expect(fileUploadIsValidContentTypeFromSettings('image/png', 'image/jpeg', '')).to.be.false;
});

it('should return false if type is defined and whiteList is not defined and type is in blackList', () => {
expect(fileUploadIsValidContentTypeFromSettings('image/jpeg', '', 'image/jpeg')).to.be.false;
});

it('should return true if type is defined and whiteList is not defined and type is not in blackList', () => {
expect(fileUploadIsValidContentTypeFromSettings('image/png', '', 'image/jpeg')).to.be.true;
});

it('should return true if type is defined and whiteList is defined and type is in whiteList with wildcard', () => {
expect(fileUploadIsValidContentTypeFromSettings('image/jpeg', 'image/*', '')).to.be.true;
});

it('should return false if type is defined and whiteList is defined and type is not in whiteList with wildcard', () => {
expect(fileUploadIsValidContentTypeFromSettings('text/plain', 'image/*', '')).to.be.false;
});

it('should return false if type is defined and whiteList is not defined and type is in blackList with wildcard', () => {
expect(fileUploadIsValidContentTypeFromSettings('image/jpeg', '', 'image/*')).to.be.false;
});

it('should return true if type is defined and whiteList is defined and type is not in blackList with wildcard', () => {
expect(fileUploadIsValidContentTypeFromSettings('text/plain', '', 'image/*')).to.be.true;
});
});

0 comments on commit 97eaa17

Please sign in to comment.