Mattermost-server: Plugin helper function `EnsureBot` doesn't compare bot details

[harshilsharma2]

Summary

Plugin helper function EnsureBot doesn't compare bot details and only checks if a bot already exists for the plugin. This causes EnsureBot to return invalid bot data when, for example, changing a bot username or trying to create multiple bots.

Steps to reproduce

1. Create a plugin that uses EnsureBot to create a bot with username foo.
2. Install and run this plugin to create the bot user foo.
3. Update the plugin and replace foo with bar in EnsureBot. Re-run the updated plugin.
4. Pring the bot ID and check which bot user does it correspond to.

Expected behavior

On running EnsureBot with an updated username, a new bot must be created with updated details and the corresponding ID be returned.

Observed behavior (that appears unintentional)

The existing bot foo's ID is returned and no new bot user is created.

Possible fixes

The problem is at this line - https://github.com/mattermost/mattermost-server/blob/0e1a9f7e530061cdd2c7c17899e458afe2c83a9b/plugin/helpers_bots.go#L234

Instead of merely checking the presence of some bot, it should verify the username or other items of the bot and ensure the requested bot is created.

[ali-farooq0]

Looks like this is indeed a bug in helpers_bots.go. And your suggestion should fix the underlying issue. Turning this into a help-wanted issue.

[ali-farooq0]

On a second thought, i think this requires a bit more discussion as EnsureBot semantics only expects to create a bot not update it. So it is working as expected. Started a discussion here for further clarification.

[ali-farooq0]

Confirmed that the semantics for EnsureBot should be of CreateOrUpdate. Turning this into a help-wanted.

[ali-farooq0]

@harshilsharma63 did you want to work on this before i make it available to public?

[harshilsharma2]

@ali-farooq0 I can pick this up over the weekend. Thanks!

[harshilsharma2]

@ali-farooq0 I've implemented the changes. Now I need to test this with an actual plugin. Should be done this week.

[ali-farooq0]

Great, thanks @harshilsharma!

[harshilsharma2]

@ali-farooq0 PR is ready and approved by @hanzei . I've tested this from an actual plugin as well and it works perfectly.