intercom
`@user` is a scary default for `user.current`
Version info
- intercom-rails version:
0.3.2 - Rails version:
4.2.0
Expected behavior
Just wanted to highlight what, in my opinion, is a risky default for this library. I do not expect Intercom to treat a user assigned to @user in a controller as the currently logged in user.
I've remedied this by explicitly setting user.current to current_user in the initializer config, but it doesn't seem like developers should have to do that. I expect assigning something to @user to have no effect on Intercom's behavior, especially since related conversations can be a big security risk. Ultimately a miss on my part, but it seems safest to not include @user as a default for user.current.
Expected: If current_user is nil and @user is defined, Intercom will not use @user as the current user.
Actual behavior
Until today I wasn't even aware that was a default in addition to current_user. It was an issue for me when displaying a profile view for a user. When viewing a user's profile (assigned to @user) without being logged in (current_user is nil), the conversations that showed up in the Intercom widget were those of that user.
Actual: If current_user is nil and @user is not, @user is used to convey currently logged in user.
Steps to reproduce
- Log out of whatever (I'm using devise)
- Visit an action that assigns
@userto the user you want to view the profile of and assume that it's one person viewing their friend Fred's profile - Open Intercom widget, see Fred's message history about how he reset his password and update his credit card information
I have the same issue. User1 started a conversation with a team, than visited the profile page of User2 and wrote to team again. All old conversation of User1 passed to User2.
- Rails version: 4.2.6
- intercom-rails version: 0.2.34
@kira4ka if you haven't already, you can fix this by adding
config.user.current = Proc.new { current_user } # or whatever your current user method is
to your Intercom initializer
@albatrocity, yes, I did it some hours ago and just finished to test it on different environments. Thanks.
We also missed that tidbit when setting up Intercom for a new customer. We were quite surprised to learn about this behaviour and the fact that we'd been leaking conversations to random people. I very much agree that it is too much of a stretch to assume that @user is the currently logged in user.
This should be higher priority, and is 100% a potential security risk. If you don't use current_user in your Proc like @albatrocity fix above, then @user STILL overrides whatever you set in the config.user.current Proc.
Our users have a public page option. Setting an @user ANYWHERE in that code causes intercom to leak @user messages out to the public. I could go in and change all our @user object names, but this library shouldn't force me to do that.
This just bit us and wanted to +1 to change this behaviour or clearly warn about it in documentation and/or logs.
