search

hubot-archive / hubot-auth

Assign roles to users and restrict command access in other scripts
149 stars 54 forks source link

admin role did not assigned correctly on 1.3.0 #37

Open siygle opened 8 years ago

siygle commented 8 years ago

After upgrade from 1.2 to 1.3, admin role seem not work correctly. I try both new variable HUBOT_AUTH_ROLES="admin=USERID" and old one HUBOT_AUTH_ADMIN=USERID and both not work.

bot always return There are no people that have the 'admin' role, I do see the log message show WARNING The HUBOT_AUTH_ADMIN environment variable has been deprecated in favor of HUBOT_AUTH_ROLES and both variable should still support on v1.3 (after I trace the source code).

Any advice? Thanks

ps: I works perfect after I downgrade to v1.2.

patcon commented 8 years ago

Sorry about that @Ferrari!

@ceci-woodward as the person who submitted the PR, cane you support this? Any thoughts what the issue might be?

@Ferrari I'm not using hubot-auth right now, so I'm not that useful here. If we def can't resolve it, and it seems a legit issue that others will hit, I'd consider reverting the changes. (but hopefully not!)

cecilia-sanare commented 8 years ago

@Ferrari I'm having a few issues reproducing this.

Can you let me know what OS and Adapter you're using?

alexleigh commented 8 years ago

I'm having similar issues where the values set in HUBOT_AUTH_ROLES aren't being picked up in 1.3.0. I'm using the Slack adapter on CoreOS (Docker container in Kubernetes.)

patcon commented 8 years ago

thanks for chiming in @ceci-woodward! if you don't have time to investigate these things this week, please make sure to let me know, and I can do a temporary revert until such a time that you have bandwidth to check it out :)

siygle commented 8 years ago

@ceci-woodward Thanks for you reply, I'm using Slack adapter & ubuntu.

thasmo commented 8 years ago

I got similar problems using 1.3.0 with Slack; U0XXXXXXX is my user id:

thasmo: list assigned roles
bot: The following roles are available: admin
thasmo: who has admin role
bot: The following people have the 'admin' role: U0XXXXXXX
thasmo: what roles do i have
bot: thasmo does not exist
liammac commented 8 years ago

This pull resolves the issue for me https://github.com/hubot-scripts/hubot-auth/pull/41

twellspring commented 8 years ago

Running in to this issue too and think the issue is in the hubot-redis-brain module. I am on hubot-redis-brain@0.0.3

I have my ID in the environment variable HUBOT_AUTH_ROLES="admin=xxxxxxxx staging=xxxxxxxx"

In hubot-auth/src/auth.coffee after line 56 I added lines to show my roles.

          user.roles.push roleName unless roleName in user.roles
          user2 = robot.brain.userForName('toddwells')
          robot.logger.info "ROLES-AUTH: #{JSON.stringify(user2.roles)}"

in hubot-redis-brain/src/redis-brain.coffee at line 48 I added the same before and after the redis merge

        user2 = robot.brain.userForName('toddwells')
        robot.logger.info "ROLES-REDIS-PRE: #{JSON.stringify(user2.roles)}"

        robot.logger.info "hubot-redis-brain: Data for #{prefix} brain retrieved from Redis"
        robot.brain.mergeData JSON.parse(reply.toString())

        user2 = robot.brain.userForName('toddwells')
        robot.logger.info "ROLES-REDIS-POST: #{JSON.stringify(user2.roles)}"

The results show that I have the expected roles before the merge but not after

[Fri Jul 29 2016 15:37:46 GMT+0000 (UTC)] INFO Connecting...
[Fri Jul 29 2016 15:37:47 GMT+0000 (UTC)] INFO Logged in as xxxxx of xxxxxxxx, but not yet connected
[Fri Jul 29 2016 15:37:47 GMT+0000 (UTC)] INFO Slack client now connected
[Fri Jul 29 2016 15:37:48 GMT+0000 (UTC)] WARNING Loading scripts from hubot-scripts.json is deprecated and will be removed in 3.0 (https://github.com/github/hubot-scripts/issues/1113) in favor of packages for each script.

Your hubot-scripts.json is empty, so you just need to remove it.
[Fri Jul 29 2016 15:37:48 GMT+0000 (UTC)] INFO ROLES-AUTH: ["admin","staging"]
[Fri Jul 29 2016 15:37:48 GMT+0000 (UTC)] INFO hubot-redis-brain: Using default redis on localhost:6379
[Fri Jul 29 2016 15:37:48 GMT+0000 (UTC)] INFO ROLES-REDIS-PRE: ["admin","staging"]
[Fri Jul 29 2016 15:37:48 GMT+0000 (UTC)] INFO hubot-redis-brain: Data for hubot brain retrieved from Redis
[Fri Jul 29 2016 15:37:48 GMT+0000 (UTC)] INFO ROLES-REDIS-POST: undefined
twellspring commented 8 years ago

That last post was a little misleading. The issue is not in hubot-redis-brain. hubot-redis-brain calls robot.brain.mergeData which is in hubot/src/brain.coffee.

Just added an issue to hubot: https://github.com/github/hubot/issues/1219

cecilia-sanare commented 8 years ago

@patcon I'd recommend just reverting the commit. The given functionality doesn't really work in the case of hubot-auth due to how the roles are stored we need to have the user defined first.

patcon commented 8 years ago

Oy. Ok, I've been MIA and feel pretty badly about this. I take full responsibility here. I'm going to revert the breaking commit, and so that this sort of thing doesn't get blocked on me again (because, as I said, i'm not using hubot-auth right now), I've given @ceci-woodward write access to the repo

cecilia-sanare commented 7 years ago

@Ferrari Can you verify if this is still an issue?