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

Security #16

Open
GioviQ opened this issue Aug 14, 2017 · 17 comments
Open

Security #16

GioviQ opened this issue Aug 14, 2017 · 17 comments

Comments

@GioviQ
Copy link

GioviQ commented Aug 14, 2017

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

@daltskin
Copy link
Collaborator

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
Copy link
Author

GioviQ commented Aug 17, 2017

Thanks

@garypretty
Copy link
Collaborator

@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
Copy link
Collaborator

@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
Copy link
Collaborator

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

Sent from my Google Nexus 5X using FastHub

@GioviQ
Copy link
Author

GioviQ commented Aug 18, 2017

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
Copy link
Owner

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
Copy link
Collaborator

garypretty commented Aug 18, 2017

@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
Copy link
Collaborator

@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
Copy link
Collaborator

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
Copy link
Collaborator

^ really need some unit tests for this :)

@tompaana
Copy link
Owner

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

@garypretty
Copy link
Collaborator

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

@tompaana
Copy link
Owner

@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
Copy link
Collaborator

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

@garypretty
Copy link
Collaborator

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

@garypretty
Copy link
Collaborator

@daltskin good timing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants
@tompaana @daltskin @garypretty @GioviQ and others