domp
domp copied to clipboard
Possible security issue
Hello,
I'm adding your gem to my app, but when going through the code, I've noticed that you use this to locate an existing user record: existing_user = current_user || User.where('email = ?', auth_params['info']['email']).first
This assumes that the email address that the provider sent you can be trusted. Couldn't a user use this to hijack another account?
Say I have a regular user Alice. She registered through my website and is not using OAuth. Bob is a hacker and he wants to be able to use Alice's account on my website. He knows her email address. He knows she doesn't have Facebook, and that you can login to the website using Facebook. So he creates a fake Facebook account using Alice's email address, and uses that Facebook account to login to Alice's account on my website. Isn't this possible?
Of course, it may seem an edge case (Facebook probably requires email verification), but since this gem supposedly supports any OAuth provider...
One possible solution to this is to require that the user be logged in before allowing him to login to an existing account with a "new" provider.
This is was I did:
authentication = provider.user_authentications.where(uid: auth_params.uid).first
existing_user = current_user || (authentication.nil? ? nil : User.where(id: authentication.user_id).first)
This basically checks if there is a UserAuthentication record between the user and the provider account.
Should the second line say User.where(id: authentication.user_id).first
instead of User.where(authentication.id).first
?
Yes you are right, I changed it.
Cool thanks!