silverbux / laravel-angular-admin

Laravel + Angularjs + Bootstrap + AdminLTE binded by Gulp workflow Admin Dashboard Boilerplate / Starter.
http://silverbux.github.io/laravel-angular-admin/
MIT License
923 stars 414 forks source link

Auth vs OAuth Conflict / Exception #76

Closed zbmowrey closed 4 years ago

zbmowrey commented 8 years ago

If a user registers using the "create an account" option and then later tries to sign in using an OAuth provider, while having the same email address in both systems, an exception is thrown since AuthController@findOrCreateUser cannot insert a record with a duplicate email address. Would it be better in this case to return to the login screen with an error indicating that the user already has an account?

zbmowrey commented 8 years ago

I have a proposed solution to this issue, but would like to discuss it; I feel like it's workable, but it doesn't make me feel super-secure.

Current behavior: On oauth callback, we check for an existing relationship with this oauth id. If found, we return the user. If not found, we create the user. This fails if the user's email is in our system, regardless of which provider was used (or even if no provider was used).

Proposed behavior: Insert an intermediary step between the oauth check and user creation, falling back to a check for the oauthUser->email. If found, update the user with this information and return. Otherwise, continue to the user creation routine.

Pro: This eliminates the wall between local accounts and oauth provider accounts, allowing a client to seamlessly switch between them with no loss of access. A user could create a local account today and login using oauth credentials tomorrow - if the email matches, the user is accessing the same account.

Con: It relies on oauth providers REQUIRING that emails be verified prior to using oauth functionality. While this is good practice -- and the three default oauths in this package do so -- extending oauth to other providers could result in a possible preemptive-registration attack (attacker registers for a service using a user's email address, then oauths to our service from that account). This attack is only possible if the attacker (a) has access to the email in question (in which case no security will prevent) or (b) is allowed to oauth prior to email confirmation.

silverbux commented 8 years ago

i was thinking of sending the user to a page which says email is already been used then offer a reset password option, it's less intrusive to users giving them option and letting them what to do next. please let me know what you feel about it.

zbmowrey commented 8 years ago

I think that's a good start, it'll let us gracefully handle the situation. Tried to implement something like this but I got another exception that the expected response was not an instance of Authenticatable, but haven't done any deep research since then.

On Wed, Oct 5, 2016 at 1:21 PM, Alex Quiambao notifications@github.com wrote:

i was thinking of sending the user to a page which says email is already been used then offer a reset password option, it's less intrusive to users giving them option and letting them what to do next. please let me know what you feel about it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/silverbux/laravel-angular-admin/issues/76#issuecomment-251740329, or mute the thread https://github.com/notifications/unsubscribe-auth/AOPWmqzqZLyaxqPIYAE7lbCPW_UhGRxbks5qw9yegaJpZM4KLi6X .