tompaana / intermediator-bot-sample

A sample bot, built with the Microsoft Bot Framework (v4), that routes messages between two users on different channels.
https://tompaana.github.io/content/chatbots_as_middlemen.html
MIT License
124 stars 65 forks source link

Security #16

Open GioviQ opened 7 years ago

GioviQ commented 7 years ago

Anyone you send "@BOT_NAME add aggregation" can become an agent?

daltskin commented 7 years ago

The DefaultBotHandler has no authorisation built in, you would have to add your own based on what security rules you needed. eg. Adding in AAD auth using https://github.com/MicrosoftDX/AuthBot and then AD group lookup. Or you may have other rules on who can be an agent based on channel, usernames etc..

GioviQ commented 7 years ago

Thanks

garypretty commented 7 years ago

@GioviQ @daltskin I have added the ability to specify permitted agent channels, so you can prevent commands from being used on non-permitted channels. You can specify a list of channel IDs in the web.config in the PermittedAgentChannels app setting. e.g. you might permit agent commands on Slack only. If the app setting value is empty or doesn't exist then all channels will accept commands

This is currently on the development branch at the moment.

daltskin commented 7 years ago

@garypretty that'll help for simple understanding of how it can be locked down. Would be worth adding in userid's for the given channel as well as all users & agents likely to be on same channel for many scenarios.

garypretty commented 7 years ago

Agreed. I'll add that later today if I get time.

Sent from my Google Nexus 5X using FastHub

GioviQ commented 7 years ago

Maybe a password request would be a better solution, when you send "command add aggregation" (mentions do not work on Telegram and Messenger). I tried also to match my Facebook ID, but Bot Framework gives me a different ID.

tompaana commented 7 years ago

Just to clarify: When I originally created the commands it was just so that it would be easy to take the bot "out for a spin". The means for managing the bot in production was left for the developer - including the security.

That said, I don't mind, at all, if we build a more secure way here as long as it doesn't make the sample harder to understand.

garypretty commented 7 years ago

@tompaana I think as long as we document any changes it will be fine. I can easily see people taking the sample and using it as is to integrate it into their bots to handle handoff, so I think it is important to be adding things like this. @daltskin thoughts?

It might be at some point that we spin out an advanced sample if it gets too complex. Then you have a simple and advanced option. That's a fairly common approach, but I think we are fine at the moment.

garypretty commented 7 years ago

@tompaana @daltskin also on the topic of security, I think I am pretty happy to just included allowed channels / agent user ids right now. The only obvious next step would be to use a signin card, but not yet :)

daltskin commented 7 years ago

I think we need to at least include a basic security option, as it is an obvious question to raise. Which is what @garypretty has worked on. But I'd like to see a sign-in card ideally. Also, need to handle one agent hijacking/taking over from another - this may be desirable in some situations on channel <-> scenario.

daltskin commented 7 years ago

^ really need some unit tests for this :)

tompaana commented 7 years ago

@garypretty @daltskin You do make an excellent point/points.

garypretty commented 7 years ago

@daltskin @tompaana what's your unit test framework of choice if we did implement any tests?

tompaana commented 7 years ago

@garypretty I don't have a personal preference and, to be honest, I've never written any tests for bots. I think this might be a good starting point: https://stackoverflow.com/a/41349523/3323695

daltskin commented 7 years ago

Some testing resources here: https://blogs.msdn.microsoft.com/jamiedalton/2017/08/07/devops-for-bots-sprinkling-some-devops-on-your-bot/

garypretty commented 7 years ago

@tompaana me neither, but there is a devops for bots series on channel 9 which discuss this as well

garypretty commented 7 years ago

@daltskin good timing!