Persist roles using brain

[dalehamel]

It looks like hubot-auth doesn't actually store the roles assigned in any persistent fashion.

This makes it relatively useless for anything but admin roles, which are provided externally.

Is this deliberate, or an oversight that user roles are not stored in the brain?

[jackwilsdon]

+1

Looks deliberate.

[patcon]

If I remeber correctly, it's storing in brain, but the way we're using hubot storage makes it unclear from the code -- specifically, we're not using set/get. This was just how it was originally written before me, and I haven't changed it.

Specifically, we have a user object from the brain here: https://github.com/hubot-scripts/hubot-auth/blob/master/src/auth.coffee#L75

And we push another user in here, which if I remember correctly, still stores it in the brain: https://github.com/hubot-scripts/hubot-auth/blob/master/src/auth.coffee#L86

If this isn't happening, then it's sheer idiocy on someone's part -- hopefully not mine, but possibly :/

The tests say it's working, fwiw (highlighted one adds "demo" role and then checks for it): https://travis-ci.org/hubot-scripts/hubot-auth/jobs/34150231 https://github.com/hubot-scripts/hubot-auth/blob/master/test/auth-test.coffee#L85-L93

Anyhow, definitely willing to accept pull requests, hopefully with better tests if it was broken :)

[dalehamel]

My test of "add a role, reboot hubot" definitely fails manually.

As far as I can tell, from the brain code only get/set are saved, as this the private data is stored as a dictionary.

I'll see if I can whip up a PR with some tests tomorrow. I only didnt do this already because I thought it was some intentional security thing.

On Monday, January 26, 2015, Patrick Connolly notifications@github.com wrote:

If I remeber correctly, it's storing in brain, but the way we're using hubot storage makes it unclear from the code -- specifically, we're not using set/get. This was just how it was originally written before me, and I haven't changed it.

Specifically, we have a user object from the brain here: https://github.com/hubot-scripts/hubot-auth/blob/master/src/auth.coffee#L75

And we push another user in here, which if I remember correctly, still stores it in the brain: https://github.com/hubot-scripts/hubot-auth/blob/master/src/auth.coffee#L86

If this isn't happening, then it's sheer idiocy on someone's part -- hopefully not mine, but possibly :/

The tests say it's working, fwiw (highlighted one adds "demo" role and then checks for it): https://travis-ci.org/hubot-scripts/hubot-auth/jobs/34150231

https://github.com/hubot-scripts/hubot-auth/blob/master/test/auth-test.coffee#L85-L93

Anyhow, definitely willing to accept pull requests, hopefully with better tests if it was broken :)

— Reply to this email directly or view it on GitHub https://github.com/hubot-scripts/hubot-auth/issues/14#issuecomment-71577199 .

[dalehamel]

I think that this has to do with implementation details. I just tried a local test with redis brain, no adapter, and it does store the role.

When i test with the slack adapter and s3-brain though, it doesn't work. This either has to do with the adapters representation of a user, or the brain's serialization method.

I'd suggest that the behavior should still be patched, but that would probably break compatibility where it is currently stored on the user object instead of in _private.

Maybe the way forward is we add an auth dictionary to _private, and detected the presence of roles in user objects for compatibility?

[chavezom]

I see the same issue. I am using redis-brain for hubot storing to Redis, but it only stores the user and not the groups.

Is there a workaround for this?

[yannh]

Hi! Interestingly, I see hubot storing the groups, but actually actively deleting the groups after a restart... Alright, after some investigation, I'm thinking the error might not be in hubot auth, considering auth stores the role properly, and the role gets removed after a restart - and I cant find anything that would do this in the module. However I am looking into the hipchat adapter and think the issue might be in this direction: https://github.com/hipchat/hubot-hipchat/blob/master/src/hipchat.coffee#L150

Do you all use the hipchat adapter?

Note: https://github.com/hipchat/hubot-hipchat/issues/213 Came across this: https://github.com/hipchat/hubot-hipchat/commit/598bace5cc8d9a5a4b053a5a3a9728c76341a7ed but it is older...

Ok it looks like the patch has been merged, but this hasn't been published to node repository, when asking for the latest hubot-hipchat I dont get the version with his patch

[yannh]

Ok my conclusion: the bug is in the hipchat adapter, and was fixed by the PR #215 https://github.com/hipchat/hubot-hipchat/pull/215 - however from what I can see the change hasn't been pushed to npmjs repository. I changed my package.json to download from git instead of from npmjs:

"hubot-hipchat": "hipchat/hubot-hipchat",

And now it works! Hope this is helpful.

[patcon]

Hooray! Good sleuthing @yannh! I wasn't seeing that behaviour, but was nonetheless feeling guilty about not putting enough time in to solve this, so I appreciate it.

I'm going to close this out for now,

[gyoza]

I am having this same problem presumably with Slack Connector, anybody have any tips on how to get this to function correctly?

[patcon]

I think auth might be in hubot core now, fyi -- been awhile since i used this, so it might be obsolete. Maybe search the core issue queue?

[cbergmann]

I have the same Problem with mattermost adapter and mysql brain. It seems to save the roles to the brain but after restart everything is resetted. Do You have any hint how you fixed it?

[teewhey]

My biggest guess is that your REDIS config is incorrect. It could be either you didn't have Redis installed or you may have misconfigured. You could check your redis and make sure your Hubot configuration points to the correct Redis location.