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.

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

[RFC] Enable bots by parsing commands in mail messages #169

Closed
dreispt opened this issue Jun 1, 2017 · 12 comments
Closed

[RFC] Enable bots by parsing commands in mail messages #169

dreispt opened this issue Jun 1, 2017 · 12 comments
Labels
enhancement help wanted question stale PR/Issue without recent activity, it'll be soon closed automatically.

Comments

@dreispt
Copy link
Sponsor Member

dreispt commented Jun 1, 2017

Example use cases:

  • Reply to a Task mail notification with a message containing something like "!!close" to trigger a server-side action closing that Task.
  • Reply to a Lead mail notification with a message containing something like "@CRMBot open amounts" to trigger a server-side reply mail with a list of the Partner open amounts.
@yajo
Copy link
Member

yajo commented Jun 1, 2017

Messages would arrive to all followers?

@dreispt
Copy link
Sponsor Member Author

dreispt commented Jun 1, 2017

Good point. Indeed that's the default behaviour.

A solution could be to "capture" messages that begin with a "@", so that these are forwarded to the corresponding bot, and committed from the Chatter wall.
Ideally, this message exchange with the bot could happen on it's own private channel.

@lasley
Copy link
Contributor

lasley commented Jun 1, 2017

How would we prevent malicious usage? A security group of people allowed to issue commands?

@dreispt
Copy link
Sponsor Member Author

dreispt commented Jun 1, 2017

@lasley You are replying to a "mail thread" - AFAIK the reply mail must have the Message ID of the thread.
But all this is plain text, and you could spy on the recipient's communications you can get that ID and impersonate him.
Or be an insider, the thread MessageID, and then forge an email from the person authorized to perform the desired action.

Maybe important operations should require a confirmation mail and reply?

@yajo
Copy link
Member

yajo commented Jun 1, 2017

Confirmation mail adds no security IMHO... odoo/odoo#11376 would be a problem, for instance.

@dreispt
Copy link
Sponsor Member Author

dreispt commented Jun 1, 2017

The issue you link to can be solved with #169 (comment)

@lasley
Copy link
Contributor

lasley commented Jun 1, 2017

But all this is plain text, and you could spy on the recipient's communications you can get that ID and impersonate him.

I'm assuming the message ID is a UUID-4, just like all the other tokens in the system. While I would argue the security behind this implementation, the system considers it secure, so we probably should too. In that light, we'd probably be fine trusting the token in the email and optionally creating a module like OCA/server-tools#835 to lock it down.

Or be an insider, the thread MessageID, and then forge an email from the person authorized to perform the desired action.

Yeah we're probably good here too. Anyone with access to the thread would have access to issue the bot commands anyways right? I think this would mean that there would be no reason to attack from this vector?

A solution could be to "capture" messages that begin with a "@", so that these are forwarded to the corresponding bot, and committed from the Chatter wall.

How would this work with HTML emails?

@dreispt
Copy link
Sponsor Member Author

dreispt commented Jun 2, 2017

Yeah we're probably good here too. Anyone with access to the thread would have access to issue the bot commands anyways right? I think this would mean that there would be no reason to attack from this vector?

Well, not quite. For example: we could have several followers, but only the Manager would have approval authorization. A resourceful follower Employee could forge a mail impersonating the Manager.

I'm aware this proposal is not perfect from a security PoV, but hopefully it is good enough for a good range of use cases.

How would this work with HTML emails?

An html2text is needed for that, but it sounds doable.
AFAICR Odoo already includes a function for that.

@yajo
Copy link
Member

yajo commented Jun 2, 2017

Well, not quite. For example: we could have several followers, but only the Manager would have approval authorization.

I only see one single use case for this proposal: being able to control Odoo through email, saving time and effort.

If the manager must approve any requests made by anybody, the time saving is lost, and then I cannot imagine a use case.

OTOH we have to keep in mind that training users to use this would be very hard. And what about translations? Not sure in your country, but here not much people speak english. Chances of getting @bot clos isue would be high, leading to a failure on the command processing and then getting either an exception or a message sent to everybody. Should we translate commands? Spanish users should use @robot cerrar incidencia? And then what happens if anybody updates the translation in Transifex, and it get synced and updated without notice? Previous commands would work no more and we'd be in the same situation again. Too much points of failure IMHO.

A resourceful follower Employee could forge a mail impersonating the Manager.

I'm aware this proposal is not perfect from a security PoV, but hopefully it is good enough for a good range of use cases.

Both assertions are true... IMHO they are enough to abort this proposal... 😞

One alternative solution that you can have is adding some buttons to mail templates. For instance, when getting a mail from an issue, you get buttons to self-assign the issue and to edit it. That is easy to use and safe, although you still have to open Odoo for that. v10 is way faster opening, so that should not be an excessive problem either. I'd go that way, honestly.

@lasley
Copy link
Contributor

lasley commented Jun 7, 2017

A resourceful follower Employee could forge a mail impersonating the Manager.

Couldn't we inspect the email headers to circumvent this, matching the user against the address in the From header?

One could reasonably assume that inbound emails have already passed spam protection (DMARC & SPF minimal), thus the sender domain is validated.

IMO domain validation is good enough, because traceability is still there. If a resourceful employee does decide to get out of line, they can be terminated. If a customer out of line, hopefully we didn't give them rights to do anything stupid. And hopefully we don't allow customers to send from our own domain.

My main worry here would be a malicious external entity attempting to issue requests under my domain, from within a message they have rights to.

OTOH we have to keep in mind that training users to use this would be very hard.

I've implemented something like this before in the past, and the solution was to halt message processing when an invalid command is detected. A helper message is sent back to the user that was attempting a command, which includes shortcut usage instructions & a copy of their original message.

That said, our system was pretty limited in scope, so it was easy enough to have an optparse type object to spit out help when needed.

In the abstract, how would we determine which shortcuts apply to which objects? From there, which objects are applicable to which mail messages?

One alternative solution that you can have is adding some buttons to mail templates

I like this proposal. What if we could make a similar shortcut mechanism, but instead pair it with the visual side of things in the templates? It seems this would nail both the security and training side of things in a pretty elegant swoop.

@dreispt
Copy link
Sponsor Member Author

dreispt commented Jun 8, 2017

I think "mail commands" can be useful for low impact actions, such as adding a Tag to an Issue (like some Github bots).
For more important actions, such as formal Approvals, action buttons on emails that link to an auth protected website page are a good solution.

So, in summary, IMO both approaches are complementary and adequate for different use cases.

@github-actions
Copy link

There hasn't been any activity on this issue in the past 6 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this issue to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted question stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

No branches or pull requests

3 participants