Mattermost-server: Implement checks on channel ID when uploading a file through the Plugin API

[mattermod]

When uploading a file to Mattermost, either through the REST API or through the Plugin API, the user needs to specify the ID of the channel the file will be uploaded to. The REST API endpoint for uploading a file makes sure that the channel ID is valid and that the user has permissions over the channel. However, the implementation in the Plugin API does not do any of these checks.

We need to make the behavior consistent across both APIs by implementing the necessary checks in the Plugin API endpoint; namely:

• check that the channel ID exists, returning an error if it does not.
• check that the user uploading the file has permissions over that channel, returning an error if the user is not authorized.

Further information:

• The entry point for the REST API endpoint is the uploadFileStream function.
• The implementation of the checks we need to clone to the Plugin API lives in the SessionHasPermissionToChannel function.
• The entry point for uploading a file through the Plugin API is the UploadFile function.

---

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

JIRA: https://mattermost.atlassian.net/browse/MM-23408

[jasonblais]

Thanks @agarciamontoro! Would you be open to adding a difficulty level as well? (via Difficulty/XXX label?

[agarciamontoro]

Done! Thank you for the heads-up, @jasonblais!

[waqasraz]

Can I give it a shot?

[agarciamontoro]

Sure, @waqasraz!

[waqasraz]

Sorry, I am little bit confused with the difficulty level
Something like this will suffice? Or I am missing something.

       _, err := a.GetChannel(channelId)
    if err != nil {
        return nil,  model.NewAppError("UploadFiles", "api.file.upload_file.incorrect_channelId.app_error",
                map[string]interface{}{"channelId": channelId}, "", http.StatusBadRequest)
    }

    if a.SessionHasPermissionToChannel(*a.Session(), channelId, model.PERMISSION_UPLOAD_FILE){
        return nil,  model.NewAppError("UploadFiles", "api.file.upload_file.user_not_authorize_upload.app_error",
            nil, "", http.StatusBadRequest)
    }

[agarciamontoro]

Hi @waqasraz. That actually seems fine! Could you implement some tests (just check that uploading a file to non-existent channels, or to channels where the user has no permissions, fails) and open a PR so we can review it and merge your changes?

And sorry if the difficulty level was not very accurate, I had not investigated further if the same implementation would suffice, so I preferred setting a higher level. If it was easier for you, much better!

[waqasraz]

that's perfect, I was going to implement the tests, I was just not sure.

Thank you