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

admin role did not assigned correctly on 1.3.0 #37

Open
siygle opened this issue Jul 11, 2016 · 12 comments
Open

admin role did not assigned correctly on 1.3.0 #37

siygle opened this issue Jul 11, 2016 · 12 comments

Comments

@siygle
Copy link

siygle commented Jul 11, 2016

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

patcon commented Jul 13, 2016

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

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

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

@alexleigh
Copy link

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

patcon commented Jul 14, 2016

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

siygle commented Jul 14, 2016

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

@thasmo
Copy link

thasmo commented Jul 28, 2016

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

liammac commented Jul 28, 2016

This pull resolves the issue for me #41

@twellspring
Copy link

Running in to this issue too and think the issue is in the hubot-redis-brain module. I am on [email protected]

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

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: hubotio/hubot#1219

@cecilia-sanare
Copy link

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

patcon commented Aug 8, 2016

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

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

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

Successfully merging a pull request may close this issue.

7 participants