search

adamwathan / eloquent-oauth

Braindead simple social login with Laravel and Eloquent.
370 stars 44 forks source link

#41 Add authorization on existing account - Laravel 4.2 #50

Closed michakpl closed 9 years ago

michakpl commented 9 years ago

I added support for already existing users without provider. Please review my commits

Thank you, Michal

adamwathan commented 9 years ago

Hey Michal thanks so much for your efforts!

Unfortunately this isn't a feature a feel comfortable supporting for two reasons.

  1. Allowing someone to connect to another account based on email address introduces a security vulnerability that I've outlined here: https://github.com/adamwathan/eloquent-oauth/issues/19#issuecomment-57561353
  2. A lot of people using this package are using it for apps where they are only supporting social login, so their users table as no email column. So we can't depend on that, and it's probably best to avoid coupling to those sorts of details as much as possible.

That being said, I would like to do some work to give people more control over how these checks happen, and I'm planning to add support for connecting two accounts in the near future, but it will be fairly limited. The plan is something like:

  1. User is already logged in with Facebook
  2. User hits "link GitHub account"
  3. Package will check to make sure that GitHub account isn't already in use.
    1. If it's unused, link the accounts
    2. If it's already been used, throw an error. Can't link because package has no idea how to combine all the various data in your app related to the two accounts.

Thanks again for the PR, sorry I can't merge it :disappointed:

michakpl commented 9 years ago

Ok, I understand, anyway thank you for quick response and code review