Merge pull request #32 from nick-woodward/master

Add a more general environment variable for adding a static set of roles

Author: patcon

README.md

@@ -28,3 +28,11 @@ Run `npm install`
 user1>> hubot user2 has jester role
 hubot>> OK, user2 has the jester role.
 ```
+
+## HUBOT_AUTH_ROLES
+
+This can be used to give a default set of roles and **must** be used to set the admin role.
+
+```sh
+HUBOT_AUTH_ROLES="admin=U12345678 mod=U87654321,U67856745"
+```

src/auth.coffee

@@ -2,7 +2,7 @@
 #   Assign roles to users and restrict command access in other scripts.
 #
 # Configuration:
-#   HUBOT_AUTH_ADMIN - A comma separate list of user IDs
+#   HUBOT_AUTH_ROLES - A list of roles with a comma delimited list of user ids
 #
 # Commands:
 #   hubot <user> has <role> role - Assigns a role to a user
@@ -25,20 +25,40 @@
 
 config =
   admin_list: process.env.HUBOT_AUTH_ADMIN
+  role_list: process.env.HUBOT_AUTH_ROLES
 
 module.exports = (robot) ->
 
-  unless config.admin_list?
-    robot.logger.warning 'The HUBOT_AUTH_ADMIN environment variable not set'
-
+  # TODO: This has been deprecated so it needs to be removed at some point.
   if config.admin_list?
-    admins = config.admin_list.split ','
+    robot.logger.warning 'The HUBOT_AUTH_ADMIN environment variable has been deprecated in favor of HUBOT_AUTH_ROLES'
+    for id in config.admin_list.split ','
+      user = robot.brain.userForId id
+
+      unless user?
+        robot.logger.warning "#{id} does not exist"
+      else
+        user.roles or= []
+        user.roles.push 'admin' unless 'admin' in user.roles
+
+  unless config.role_list?
+    robot.logger.warning 'The HUBOT_AUTH_ROLES environment variable not set'
   else
-    admins = []
+    for role in config.role_list.split ' '
+      [dummy, roleName, userIds] = role.match /(\w+)=([\w]+(?:,[\w]+)*)/
+      for id in userIds.split ','
+        user = robot.brain.userForId id
+
+        unless user?
+          robot.logger.warning "#{id} does not exist"
+        else
+          user.roles or= []
+          user.roles.push roleName unless roleName in user.roles
 
   class Auth
     isAdmin: (user) ->
-      user.id.toString() in admins
+      roles = robot.brain.userForId(user.id).roles or []
+      'admin' in roles
 
     hasRole: (user, roles) ->
       userRoles = @userRoles(user)
@@ -50,18 +70,13 @@ module.exports = (robot) ->
 
     usersWithRole: (role) ->
       users = []
-      for own key, user of robot.brain.data.users
+      for own key, user of robot.brain.users()
         if @hasRole(user, role)
           users.push(user.name)
       users
 
     userRoles: (user) ->
-      roles = []
-      if user? and robot.auth.isAdmin user
-        roles.push('admin')
-      if user.roles?
-        roles = roles.concat user.roles
-      roles
+      user.roles
 
   robot.auth = new Auth
 
@@ -84,7 +99,7 @@ module.exports = (robot) ->
           msg.reply "#{name} already has the '#{newRole}' role."
         else
           if newRole is 'admin'
-            msg.reply "Sorry, the 'admin' role can only be defined in the HUBOT_AUTH_ADMIN env variable."
+            msg.reply "Sorry, the 'admin' role can only be defined in the HUBOT_AUTH_ROLES env variable."
           else
             myRoles = msg.message.user.roles or []
             user.roles.push(newRole)
@@ -105,7 +120,7 @@ module.exports = (robot) ->
         user.roles or= []
 
         if newRole is 'admin'
-          msg.reply "Sorry, the 'admin' role can only be removed from the HUBOT_AUTH_ADMIN env variable."
+          msg.reply "Sorry, the 'admin' role can only be removed from the HUBOT_AUTH_ROLES env variable."
         else
           myRoles = msg.message.user.roles or []
           user.roles = (role for role in user.roles when role isnt newRole)
@@ -137,7 +152,7 @@ module.exports = (robot) ->
     unless robot.auth.isAdmin msg.message.user
         msg.reply "Sorry, only admins can list assigned roles."
     else
-        for i, user of robot.brain.data.users when user.roles
+        for i, user of robot.brain.users() when user.roles
             roles.push role for role in user.roles when role not in roles
         if roles.length > 0
             msg.reply "The following roles are available: #{roles.join(', ')}"

test/auth-test.coffee

@@ -6,7 +6,7 @@ expect = require("chai").expect
 describe "auth", ->
 
   beforeEach ->
-    process.env.HUBOT_AUTH_ADMIN = "alice"
+    process.env.HUBOT_AUTH_ROLES = "admin=alice"
     @room = helper.createRoom()
     @room.robot.brain.userForId "alice",
       name: "alice"
@@ -42,7 +42,7 @@ describe "auth", ->
       @room.user.say("alice", "hubot: jimmy has admin role").then =>
         expect(@room.messages).to.eql [
           ["alice", "hubot: jimmy has admin role"]
-          ["hubot", "@alice Sorry, the 'admin' role can only be defined in the HUBOT_AUTH_ADMIN env variable."]
+          ["hubot", "@alice Sorry, the 'admin' role can only be defined in the HUBOT_AUTH_ROLES env variable."]
         ]
 
     it "anon user fails to set role", ->
@@ -67,7 +67,7 @@ describe "auth", ->
       @room.user.say("alice", "hubot: jimmy doesn't have admin role").then =>
         expect(@room.messages).to.eql [
           ["alice", "hubot: jimmy doesn't have admin role"]
-          ["hubot", "@alice Sorry, the 'admin' role can only be removed from the HUBOT_AUTH_ADMIN env variable."]
+          ["hubot", "@alice Sorry, the 'admin' role can only be removed from the HUBOT_AUTH_ROLES env variable."]
         ]
 
   context "what roles does <user> have", ->
@@ -114,5 +114,5 @@ describe "auth", ->
             ["alice", "hubot: alice has test role"]
             ["hubot", "@alice OK, alice has the 'test' role."]
             ["alice", "hubot: list assigned roles"]
-            ["hubot", "@alice The following roles are available: demo, test"]
+            ["hubot", "@alice The following roles are available: admin, demo, test"]
           ]