Regression: File upload via apps not working in some scenarios

[shiqimei]

Proposed changes

✅ Fix errorClass [Error]: Forbidden [forbidden]

In app/file-upload/lib/FileUploadBase.js, we configured UploadFS.config.defaultStorePermissions, which validates insert(userId, doc) etc. But the parameter userId can't be always obtained by this validation method correctly (sometimes it's undefined). Meteor use the Meteor.userId() as its fallback option. So we can wrap the original call with Meteor.runAsUser to solve issue.

✅ Add a new validator into canAccessRoom

canAccessRoom (app/authorization/server/functions/canAccessRoom.js) is an essential validator for Rocket.Chat to check whether some user has permissions to access some room. In this PR, we added a new validator that allows app users to access any room on a Rocket.Chat server even if it is not a member of the room.

canAcessRoom is called "everywhere", for many different purposes (not only send files or messages), so adding a bypass inside canAccessRoom can potentially allow apps to do stuff we're not prepared (yet).

✅ Attempt to fix Meteor code must always run within a Fiber Error

Original Error: "Error: Meteor code must always run within a Fiber. Try wrapping callbacks that you pass to non-Meteor libraries with Meteor.bindEnvironment."

✅ Support for uploading files by a Livechat visitor

Issue(s)

https://open.rocket.chat/group/rocketchat-apps-internal?msg=yms4Km3wundZuQtfn (Private)

How to test or reproduce

The following behaviors shouldn't be corrupted

• Upload via the livechat widget
• Upload via a browser

The following behaviors should be working well

• Apps: Upload via a livechat agent
• Apps: Upload via a livechat visitor
• Apps: Upload via an app user
• Apps: Upload within HTTP endpoints handlers
• Apps: Upload within message event handlers

Upload files using the default app user

import { Buffer } from 'buffer';

import { IAppAccessors, IHttp, ILogger, IModify, IPersistence, IRead } from '@rocket.chat/apps-engine/definition/accessors';
import { App } from '@rocket.chat/apps-engine/definition/App';
import { ILivechatRoom, IPostLivechatRoomStarted } from '@rocket.chat/apps-engine/definition/livechat';
import { IAppInfo } from '@rocket.chat/apps-engine/definition/metadata';

export class FileUploadApp extends App implements IPostLivechatRoomStarted {
    constructor(info: IAppInfo, logger: ILogger, accessors: IAppAccessors) {
        super(info, logger, accessors);
    }

    public async executePostLivechatRoomStarted(room: ILivechatRoom, read: IRead, http: IHttp, persis: IPersistence, modify?: IModify): Promise<void> {
        try {
            if (!modify) {
                return;
            }

            for (const filename of Array(5).fill(Math.random())) {
                await modify.getCreator().getUploadCreator().uploadBuffer(Buffer.from('Hello World', 'utf-8'), {
                    filename: `${ filename }.txt`,
                    room,
                });
            }
        } catch (err) {
            console.error(err);
        }
    }
}

fileupload_0.0.1.zip

Upload files using a Livechat agent

import { Buffer } from 'buffer';

import { IAppAccessors, IHttp, ILogger, IModify, IPersistence, IRead } from '@rocket.chat/apps-engine/definition/accessors';
import { App } from '@rocket.chat/apps-engine/definition/App';
import { ILivechatRoom } from '@rocket.chat/apps-engine/definition/livechat';
import { IMessage, IPostMessageSent } from '@rocket.chat/apps-engine/definition/messages';
import { IAppInfo } from '@rocket.chat/apps-engine/definition/metadata';
import { RoomType } from '@rocket.chat/apps-engine/definition/rooms';

export class FileUploadApp extends App implements IPostMessageSent {
    constructor(info: IAppInfo, logger: ILogger, accessors: IAppAccessors) {
        super(info, logger, accessors);
    }

    public async executePostMessageSent(message: IMessage, read: IRead, http: IHttp, persistence: IPersistence, modify: IModify): Promise<void> {
        try {
            if (!message.text || message.room.type !== RoomType.LIVE_CHAT) {
                return;
            }
            const agent = (message.room as ILivechatRoom).servedBy;

            for (const filename of Array(3).fill(Math.random())) {
                await modify.getCreator().getUploadCreator().uploadBuffer(Buffer.from('Hello World', 'utf-8'), {
                    filename: `${ filename } sent by ${ agent && agent.name }.txt`,
                    room: message.room,
                    user: agent,
                });
            }
        } catch (err) {
            console.error(err);
        }
    }
}

fileupload_0.0.1.zip

Upload files within an HTTP endpoint handler

// endpoint.ts
import { Buffer } from 'buffer';

import { HttpStatusCode, IHttp, IModify, IPersistence, IRead } from '@rocket.chat/apps-engine/definition/accessors';
import { ApiEndpoint, IApiEndpointInfo, IApiRequest, IApiResponse } from '@rocket.chat/apps-engine/definition/api';
import { IUpload } from '@rocket.chat/apps-engine/definition/uploads';

export class Endpoint extends ApiEndpoint {
    public path = 'api';

    public async post(
        request: IApiRequest, endpoint: IApiEndpointInfo, read: IRead, modify: IModify, http: IHttp, persis: IPersistence,
    ): Promise<IApiResponse> {
        const room = await read.getRoomReader().getByName('general');

        if (!room) {
            return {
                status: HttpStatusCode.NOT_FOUND,
                content: `Room "#general" could not be found`,
            };
        }

        try {
            const uploadedFiles = [] as Array<IUpload>;

            for (const filename of Array(10).fill(Math.random())) {
                const uploadedFile = await modify.getCreator().getUploadCreator().uploadBuffer(Buffer.from('Hello World', 'utf-8'), {
                    filename: `${ filename }.txt`,
                    room,
                });
                uploadedFiles.push(uploadedFile);
            }
            return this.success(JSON.stringify(uploadedFiles, null, 4));
        } catch (err) {
            console.error(err);
            return this.json({
                status: HttpStatusCode.OK,
                content: {
                    succuess: false,
                },
            });
        }
    }
}

fileupload_0.0.1.zip

Screenshots

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• Improvement (non-breaking change which improves a current function)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Hotfix (a major bugfix that has to be merged asap)
• Documentation Update (if none of the other choices apply)

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA
• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works (if applicable)
• I have added necessary documentation (if applicable)
• Any dependent changes have been merged and published in downstream modules

Changelog

Further comments

[d-gubert]

⬜️ Attempt to fix Meteor code must always run within a Fiber Error

Original Error: "Error: Meteor code must always run within a Fiber. Try wrapping callbacks that you pass to non-Meteor libraries with Meteor.bindEnvironment."

I think it's important to note that neither me or @lolimay were able to reproduce this error in our dev env, but @renatobecker and @murtaza98 were. The approach applied in this PR does not appear to differ from what the FileUpload already does.

Even though this might solve the problem, we still don't know what the problem is, and need to investigate further

[d-gubert]

After further investigation, we came two a state where could pinpoint that

• apps attempting to upload files in response to hook triggers (e.g., IPostMessageSent, IPreRoomCreatePrevent, etc.), like the example app we were using, succeed in doing so;
• apps attempting to upload files in response to HTTP request, via their own registered endpoints ( ApiEndpoint ), do not succeed, failing with the message "Error: Meteor code must always run within a Fiber. Try wrapping callbacks that you pass to non-Meteor libraries with Meteor.bindEnvironment.".

Given this evidence, we suspect that the code triggering the app's internal HTTP handlers is not properly setting up the environment for execution of Meteor code, but we couldn't identify where in the call stack the problem is.

@lolimay's approach is spot on in solving this problem, as it ensures the environment will be set correctly, even if it was already in the right state earlier.

At the end of the day, I blame this one on Meteor 😛

[shiqimei]

Depends on: RocketChat/Rocket.Chat.Apps-engine#323

[shiqimei]

they are not equivalent here 😛

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