intermediator-bot-sample icon indicating copy to clipboard operation
intermediator-bot-sample copied to clipboard

Security

Open GioviQ opened this issue 8 years ago • 17 comments

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

GioviQ avatar Aug 14 '17 06:08 GioviQ

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..

daltskin avatar Aug 17 '17 08:08 daltskin

Thanks

GioviQ avatar Aug 17 '17 13:08 GioviQ

@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.

garypretty avatar Aug 18 '17 07:08 garypretty

@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.

daltskin avatar Aug 18 '17 08:08 daltskin

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

Sent from my Google Nexus 5X using FastHub

garypretty avatar Aug 18 '17 08:08 garypretty

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.

GioviQ avatar Aug 18 '17 08:08 GioviQ

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.

tompaana avatar Aug 18 '17 10:08 tompaana

@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 avatar Aug 18 '17 10:08 garypretty

@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 :)

garypretty avatar Aug 18 '17 10:08 garypretty

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 avatar Aug 18 '17 10:08 daltskin

^ really need some unit tests for this :)

daltskin avatar Aug 18 '17 10:08 daltskin

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

tompaana avatar Aug 18 '17 10:08 tompaana

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

garypretty avatar Aug 18 '17 10:08 garypretty

@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

tompaana avatar Aug 18 '17 10:08 tompaana

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

daltskin avatar Aug 18 '17 10:08 daltskin

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

garypretty avatar Aug 18 '17 10:08 garypretty

@daltskin good timing!

garypretty avatar Aug 18 '17 10:08 garypretty