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.

Sign up for GitHub

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

Regression: File upload via apps not working in some scenarios #18995

Merged
merged 10 commits into from
Sep 29, 2020
Merged

Regression: File upload via apps not working in some scenarios #18995

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
089bc3e
[FIX] [Apps] Fix app user doesn't has permission to upload files
shiqimei Sep 23, 2020
0a9ff96
Add support for uploading files by a livecaht visitor
shiqimei Sep 23, 2020
1060339
Support upload files with livechat visitors
shiqimei Sep 24, 2020
4b87683
Reduce an unnecessary DB query - Users.findOneById
shiqimei Sep 24, 2020
9b86d55
Move the "bypass" out of canAccessRoom
shiqimei Sep 25, 2020
c0538f3
Merge remote-tracking branch 'origin/develop' into apps/fix.file-upload
d-gubert Sep 28, 2020
79cc59e
Update Apps-Engine version
d-gubert Sep 28, 2020
259af21
Some refactoring
d-gubert Sep 28, 2020
4eccb96
Merge remote-tracking branch 'origin/develop' into apps/fix.file-upload
d-gubert Sep 28, 2020
bb5ad5f
Fix a rateada
d-gubert Sep 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 29 additions & 9 deletions app/apps/server/bridges/uploads.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,40 @@ export class AppUploadBridge {
});
}

createUpload(details, buffer, appId) {
async createUpload(details, buffer, appId) {
this.orch.debugLog(`The App ${ appId } is creating an upload "${ details.name }"`);

details.type = determineFileType(buffer, details);
if (!details.userId && !details.visitorToken) {
throw new Error('Missing user to perform the upload operation');
}

if (details.visitorToken) {
delete details.userId;
}

const fileStore = FileUpload.getStore('Uploads');
const uploadedFile = fileStore.insertSync(details, buffer);
const insertSync = details.userId
? (...args) => Meteor.runAsUser(details.userId, () => fileStore.insertSync(...args))
: Meteor.wrapAsync(fileStore.insert.bind(fileStore));

if (details.userId) {
Meteor.runAsUser(details.userId, () => {
Meteor.call('sendFileMessage', details.rid, null, uploadedFile);
});
}
details.type = determineFileType(buffer, details);

return new Promise(Meteor.bindEnvironment((resolve, reject) => {
try {
const uploadedFile = insertSync(details, buffer);

if (details.visitorToken) {
Meteor.call('sendFileLivechatMessage', details.rid, details.visitorToken, uploadedFile);
} else {
Meteor.runAsUser(details.userId, () => {
Meteor.call('sendFileMessage', details.rid, null, uploadedFile);
});
}

return this.orch.getConverters().get('uploads').convertToApp(uploadedFile);
resolve(this.orch.getConverters().get('uploads').convertToApp(uploadedFile));
} catch (err) {
reject(err);
}
}));
}
}
2 changes: 1 addition & 1 deletion app/file-upload/server/lib/FileUpload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export const FileUpload = {
const room = Rooms.findOneById(file.rid);
const directMessageAllowed = settings.get('FileUpload_Enabled_Direct');
const fileUploadAllowed = settings.get('FileUpload_Enabled');
if (user.type !== 'app' && canAccessRoom(room, user, file) !== true) {
if (user?.type !== 'app' && canAccessRoom(room, user, file) !== true) {
return false;
}
const language = user ? user.language : 'en';
Expand Down
8 changes: 5 additions & 3 deletions app/file-upload/server/methods/sendFileMessage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,21 @@ import { Random } from 'meteor/random';
import _ from 'underscore';

import { Uploads } from '../../../models';
import { Rooms } from '../../../models/server/raw';
import { callbacks } from '../../../callbacks';
import { FileUpload } from '../lib/FileUpload';
import { canAccessRoom } from '../../../authorization/server/functions/canAccessRoom';

Meteor.methods({
async sendFileMessage(roomId, store, file, msgData = {}) {
if (!Meteor.userId()) {
throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'sendFileMessage' });
}

const room = Meteor.call('canAccessRoom', roomId, Meteor.userId());
const room = await Rooms.findOneById(roomId);
const user = Meteor.user();

if (!room) {
if (user?.type !== 'app' && !canAccessRoom(room, user) !== true) {
return false;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they are not equivalent here 😛

> var user = null
undefined
> !!(user && user.type !== 'app')
false
> !!(user?.type !== 'app')
true
>


Expand Down Expand Up @@ -65,7 +68,6 @@ Meteor.methods({
attachment.video_size = file.size;
}

const user = Meteor.user();
let msg = Object.assign({
_id: Random.id(),
rid: roomId,
Expand Down
58 changes: 9 additions & 49 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
"@nivo/heatmap": "^0.61.0",
"@nivo/line": "^0.61.1",
"@nivo/pie": "^0.61.1",
"@rocket.chat/apps-engine": "1.18.0-beta.3848",
"@rocket.chat/apps-engine": "1.18.0-beta.3873",
"@rocket.chat/css-in-js": "^0.14.1",
"@rocket.chat/fuselage": "^0.14.1",
"@rocket.chat/fuselage-hooks": "^0.14.1",
Expand Down