Steps to reproduce
Imagine:
Alice has a Facebook account, no account yet on your Drupal site
Bob has a Drupal account on your website, and no Facebook account
You need a test Facebook account for Alice, that has never logged in your Drupal site yet. Alternatively, to simulate new user, take your own Facebook account, and delete your site's app in Facebook App Settings, or edit them and remove permission for e-mail.
- Log out from both Drupal and Facebook
- Use Simple FB Connect to sign up, with Alice account
- Deny permission for e-mail during the OAuth flow
- Error message is displayed about missing e-mail permission
- Login with Bob account
Actual
In his $_SESSION variable, Bob has the Facebook token from Alice, in $_SESSION['simple_fb_connect']['user_token']. This token necessarily has e-mail permission denied (because of error displayed in 4.), but may have any count of other scopes granted, depending on what scopes were requested.
Now, any code that checks presence of this session data and uses it will make Bob act as Alice on Facebook.
Expected
When Bob logs in, he has no simple_fb_connect session data.
Solution
In case of error during the Facebook login flow, simple_fb_connect must delete the session data.
Other possible way: implement hook_user_login() and unset there session data from Simple FB Connect when the login did not happen through Simple FB Connect.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 0001-Do-not-store-Facebook-tokens-in-Drupal-anonymous-use.patch | 4.33 KB | francoisb |
Comments
Comment #2
francoisb commentedHere's the fix. It deletes Facebook token from PHP session data for the Drupal anonymous user, when the Facebook login fails.
To verify, please follow the "Steps to reproduce" in issue summary, with and without this patch. At the end, inspect the $_SESSION in any Drupal page with
dpm($_SESSION);if you have devel module enabled, otherwise with
drupal_set_message(print_r($_SESSION, TRUE));Comment #3
francoisb commentedComment #4
francoisb commentedAnother possible way to fix this issue: implement
hook_user_login()and unset there session data from Simple FB Connect when the login did not happen through Simple FB Connect. What do you think?Comment #5
francoisb commentedComment #7
masipila commentedCommitted to 7.x-2.x-dev with two changes to the proposed patch:
Thank you for your work with this issue!
Cheers,
Markus
Comment #8
masipila commentedComment #9
masipila commented