groupby / hubot-hangups

Hubot adapter using hangups
39 stars 3 forks source link

Need some basic security #12

Open ehacke opened 9 years ago

ehacke commented 9 years ago

At the moment, if anyone adds a hubot with this adapter to their hangout, they can immediately start sending commands.

Need a layer of security in this adapter that allows people with an admin role to add other people to a whitelist that hubot will respond to. And we'll add the ability to turn off this feature for those that don't want it or don't care.

Additional per-command security should be added inside of hubot.

codeallthethingz commented 9 years ago

@ehacke should this go in R2?

ehacke commented 9 years ago

@willwarren I was thinking that the per-command security should be handled there. But it might be a good feature to have an overall whitelist in the hangouts adapter.

So there would be the basic level of security in hubot-hangups that just whitelists whole groups of people. (everyone with a @groupbyinc.com address for example) and then a per-command security in r2d2.

bubblemanss commented 9 years ago

Currently Hangups does not populate its email field so we can't use emails for whitelisting

nylonee commented 9 years ago

An idea which I've been thinking of implementing in Hangupsbot is to set a 'role' to each user eg. 'Member', 'Guest', 'Admin', 'Superuser' and people can run commands according to role. Just an idea.

On Thu Jan 29 2015 at 5:24:17 AM l3ubbleman notifications@github.com wrote:

Currently Hangups does not populate its email field so we can't use emails for whitelisting

— Reply to this email directly or view it on GitHub https://github.com/groupby/hubot-hangups/issues/12#issuecomment-71919044 .

ehacke commented 9 years ago

@nylonee In our case it probably makes more sense to use something like the hubot auth script for restricting the commands on a per-user basis. That's already built, and probably works well enough.

For our application in our company, anyone that should be using this has the same domain name in their email. So I was hoping to just whitelist email domain names. And then give a few people beyond that admin rights using the auth script. That way we don't have to set a role/whitelist every single individual user.

However, it looks like hangups doesn't populate the email field of the user object, so we can't see the email at the moment. So either I have to change the parser in Hangups to process the email address, or rely just on a per user role in the auth script.

codeallthethingz commented 9 years ago

@ehacke We definitely need to set a role per user.
For example,

I think this should go in the r2 layer using the auth script as you suggested.

ehacke commented 9 years ago

@willwarren absolutely, I just was hoping of making it implicit rather than explicit.

For example:

Thing is, doesn't matter at the moment because there is no way to filter emails if we can't read the email addresses. So everything will have to be done in the auth script anyways with explicit roles for everyone.