Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Accepted Media Types settings validation #32478

Merged
merged 8 commits into from
Jun 14, 2024
Merged
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
16 changes: 12 additions & 4 deletions apps/meteor/app/utils/lib/restrictions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
if (!mediaTypeWhiteList || mediaTypeWhiteList === '*') {
return;
}
return _.map(mediaTypeWhiteList.split(','), (item) => {

Check warning on line 9 in apps/meteor/app/utils/lib/restrictions.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint

Consider using the native Array.prototype.map()
return item.trim();
});
};
Expand All @@ -17,7 +17,7 @@
return;
}

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

Check warning on line 20 in apps/meteor/app/utils/lib/restrictions.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint

Consider using the native Array.prototype.map()
};

const isTypeOnList = function (type: string, list: string[]): boolean | undefined {
Expand All @@ -33,21 +33,29 @@
}
};

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)) {
if (whiteList && type && isTypeOnList(type, whiteList)) {
return true;
}

if (!type && whiteList) {
return false;
}
hugocostadev marked this conversation as resolved.
Show resolved Hide resolved

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;
});
});
Loading