Skip to content

[Regression] Omnichannel visitors not able to upload files #19429

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

Closed
murtaza98 wants to merge 4 commits into from
Closed

[Regression] Omnichannel visitors not able to upload files #19429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0b9cc7f
Fix errors when vistors uploads file via apps-engine
murtaza98 Nov 1, 2020
c492e13
Allow file uploads from omnichannel visitors
murtaza98 Nov 1, 2020
0674e6e
revert unnecessary changes
murtaza98 Nov 2, 2020
d7ad2ff
Merge branch 'develop' into fix-visitor-upload-errors
murtaza98 Nov 8, 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
5 changes: 5 additions & 0 deletions app/file-upload/lib/FileUploadBase.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ UploadFS.config.defaultStorePermissions = new UploadFS.StorePermissions({
return true;
}

// allows inserts from omnichannel visitors
if (doc.visitorToken) {
Copy link
Member

@sampaiodiego sampaiodiego Nov 8, 2020

Choose a reason for hiding this comment

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

you should not allow any document just because if has a visitorToken, it could lead to undesired results..

I think the permission validation was fixed by #19468

can you please test again removing this code and getting the latest changes from develop?

Copy link
Contributor Author

@murtaza98 murtaza98 Nov 8, 2020

Choose a reason for hiding this comment

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

Thanks @sampaiodiego It seems that the latest changes on develop branch have fixed all the above error which I was facing.

Its also worth mentioning that this change is also not required, since the of this validator which canAccessRoom calls returns true if the visitor belongs to the same room.

return true;
}

// allow inserts from slackbridge (message_id = slack-timestamp-milli)
if (doc && doc.message_id && doc.message_id.indexOf('slack-') === 0) {
return true;
Expand Down
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 && user.type !== 'app' && canAccessRoom(room, user, file) !== true) {
Copy link
Contributor

@shiqimei shiqimei Nov 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@d-gubert I believe this bug was introduced in this commit 259af21#diff-2b708d47a45f905401f41f0b318f2462e95eebf42d8e049bc5474a6f65d2e779R69

return false;
}
const language = user ? user.language : 'en';
Expand Down