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

Redis brain data seems to be missing Slack Names - hubot-auth related #326

Closed
3 tasks done
mechastorm opened this issue Jul 21, 2016 · 16 comments
Closed
3 tasks done
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented

Comments

@mechastorm
Copy link

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.

Description

The redis brain data for user seems to be incomplete. I need to clarify if this is possibly a hubot-slack issue or more a hubot-auth issue.

The brain data seems to be incomplete for users. I am only get partial number of users (not all). Seems to be mostly just the users of the room it first joined. I assume this is expected?

But the user data only contains user ids, even the names are just user IDs at time. I try to flush the redis data and restart hubot to rebuild it but then I end up it not getting the user data at all.

"{\"users\":{},\"_private\":{\"roles\":{\"\\\"U1Q2S8DRT\":[\"admin\"],\"U1Q0YUDEF\\\"\":[\"admin\"],\"U1Q2S8DRT\":[\"admin\"],\"U1Q0YUDEF\":[\"admin\"]}}}"

Its like Hubot did not make the Slack API call to obtain the user list.

My Package JSON

{
  "name": "publisher-bot",
  "version": "0.0.0",
  "private": true,
  "author": "Publisher <publisher.campaigns>",
  "description": "Publisher",
  "dependencies": {
    "hubot": "^2.19.0",
    "hubot-auth": "^1.3.0",
    "hubot-diagnostics": "0.0.1",
    "hubot-help": "^0.2.0",
    "hubot-redis-brain": "0.0.3",
    "hubot-scripts": "^2.17.2",
    "hubot-slack": "^4.0.1"
  },
  "engines": {
    "node": "0.10.x"
  }
}

Reproducible in:

{project_name} version: 4.3.1
OS version(s): Ubuntu 14.04

Expected result:

Redis brain data will contain slack user IDs and proper name.

  • Name should be the proper name of the users
  • Should have a full list of users

Actual result:

  • only a few users show up
  • names are just the slack user IDs
@DEGoodmanWilson DEGoodmanWilson added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented Priority—Medium labels Jul 21, 2016
@nsturnitin
Copy link

Yes, broke for me as well: hubot-archive/hubot-auth#42

@kuba-kubula
Copy link

also found the issue with hubot.brain not being used.

@moos3
Copy link

moos3 commented Aug 22, 2016

I think this should get bump from medium to high. As is breaks any form of authentication when using slack and leaves it completely open or unusable commands.

@rattboi
Copy link

rattboi commented Sep 12, 2016

This also breaks hubot-rbac. Same kind of issue, obviously.

@DEGoodmanWilson
Copy link

Bumping to high. Sorry this keeps getting delayed.

@Craigeous
Copy link

it looks like it's using the RTM datastore at the bottom of the script... this was not in the old code posted above.

https://github.com/slackhq/hubot-slack/blob/master/src/bot.coffee#L170

@telemaco
Copy link

Is there any patch or workarround about this?

@asalaheldin
Copy link

asalaheldin commented Nov 7, 2016

The only workaround I could found so far as nick suggest here is to revert hubot-auth to version v1.2.0 and hubot-slack to v3.4.2 till a fix is done for this. The steps I followed are as follow
1- npm uninstall --save hubot-slack
2- npm uninstall --save hubot-auth
3- npm install --save [email protected]
4- npm install --save [email protected]

Hope a fix is available soon for versions after 3.4.2

@triwats
Copy link

triwats commented Nov 14, 2016

Can that the roll back mentioned by @asalaheldin works as a workaround for this problem for the time being, ran into it today with the hubot-keys script.

@vikramshinde12
Copy link

workaround mentioned by @asalaheldin worked. Thanks.
I am using hubot-auth and slack.
Thanks

@jianyuan
Copy link

jianyuan commented Dec 4, 2016

Temporary workaround to re-instantiate Slack names.

I'm calling Slack's users.list() web API method to retrieve a list of users.

robot.brain.on 'loaded', (data) ->
  return unless robot.adapterName == 'slack'

  robot.adapter.client.web.users.list (err, info) ->
    if err
      robot.logger.debug 'Cannot retrieve list of users'
      return

    for member in info.members
      continue unless member?.id?
      newUser =
        name: member.name
        realName: member.real_name
        slack: member
      delete robot.brain.data.users[member.id]
      robot.brain.userForId member.id, newUser

nickstenning added a commit to hypothesis/vannevar that referenced this issue Dec 12, 2016
The hubot-auth module doesn't work very well in combination with
hubot-slack, mainly because of

  slackapi/hubot-slack#326

We don't make extensive use of hubot-auth roles, so replace them with a
thin wrapper over the user profile data provided by Slack so that we can
distinguish between admins/non-admins and staff/guests.
@jivany
Copy link

jivany commented Dec 14, 2016

Careful with the above workaround - by deleting the existing user info in the brain you will destroy any values already saved. For example - the hubot-auth list of roles that a user has. Or at least that was my experience.

@ferrixh
Copy link

ferrixh commented Jan 18, 2017

Does #381 fix this? Does an upgrade mess up the brain?

@asalaheldin
Copy link

As referenced by @sebastiandero and @ferrixh the #381 seems to be the fix of our issue here. I have just tried this and the users data have been loaded inside Hubot brain without any workaround just applied the new version of "hubot-slack" and "hubot-auth". Thanks a lot for mentioning this fix here guys

@aoberoi
Copy link
Contributor

aoberoi commented Oct 18, 2017

it sounds like the latest word is that this issue is fixed. if anyone finds evidence that its still not working correctly, please leave a comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented
Projects
None yet
Development

No branches or pull requests