devise_token_auth icon indicating copy to clipboard operation
devise_token_auth copied to clipboard

Allow multiple openauth providers

Open ACPK opened this issue 9 years ago • 24 comments

In my app, I'm using a seperate model for openauth keys so that a user can connect with multiple providers. Will this be something that you'll be supporting down the road?

ACPK avatar Sep 04 '14 04:09 ACPK

I'm open to supporting multiple OAuth providers, but I'm worried that problems will arise that may not have a definitive solution. For example:

  • How will users go about associating their accounts?
  • Which account settings will take precedence?
  • If a user has already registered separate accounts using different OAuth providers (i.e. Twitter and Facebook), and now the user wants to merge their Twitter account into their Facebook account, how do we proceed? Is the user still allowed to keep the separate accounts? If not, what do we do with any data that was associated with the old account?

I can think of different use-cases that would require different solutions to these problems. I guess my question is, is there a single best way to do this?

lynndylanhurley avatar Sep 04 '14 06:09 lynndylanhurley

After some thought, here is my proposal for this feature. I can think of two possible approaches to this, which I'm calling "Plan A" and "Plan B".

Plan A

  1. Account creation / authentication continues to exist in its current state. Users will continue use a single auth provider (OAuth or email) to sign-in.
  2. A new model will be created for additional 3rd party accounts. This model will have a belongs_to relationship with the original User model.

This has the following benefits:

  • Users will have access to their various accounts. For example:
    • A user that authenticated using Github will be able to easily import their LinkedIn data.
    • A user that signed up using email could easily access their facebook photos.
  • Sign-up and sign-in remain simple and performant (faster than Plan B).

This plan has the following drawbacks:

  • Users will only be able to sign-in with a single account.
  • If a user de-activates the provider account that they use for sign-in, they will be locked out of the app.

Plan B

An alternative plan would be to use belongs_to associations for all providers.

  1. All providers (email + OAuth) exist as belongs_to associations of a core User model.
  2. Authentication will be done against the provider models instead of the core User model. This will allow login via multiple provider accounts (OAuth + email).
  3. The original User model is reduced to a unique uid column. The model will used only to bind the various provider models together via has_many associations.

This plan has the following benefits:

  • Users can sign-in using multiple accounts.
  • If a user de-activates one of their provider accounts (i.e. deletes their facebook profile), the user could still sign in using an alternative account.

But this plan has worse drawbacks in my opinion:

  • 99% of users will only use a single account. This plan increases the complexity of the most common use case, and is slightly less performant due to excessive association lookups.
  • It's not exactly clear how to merge the accounts in terms of UX. Here is one un-resolvable issue for example:
    • What if a user creates separate, isolated accounts using multiple providers, but then wants to merge the accounts? Will this be possible, or is the user stuck with two separate accounts?
    • What if a user creates a merged account, but then wants to break them apart into isolated accounts? Is this possible, or is the user stuck with merged accounts?
  • This will probably break compatibility with previous versions.

I can see some benefit to "Plan A", and I hate "Plan B". I'll leave this open for a while to see if anyone has any comments before I get started.

lynndylanhurley avatar Sep 30 '14 18:09 lynndylanhurley

I used this method in the past to hook up multiple providers on one account and it worked really well:

http://sourcey.com/rails-4-omniauth-using-devise-with-twitter-facebook-and-linkedin/

skyriverbend avatar Oct 21 '14 09:10 skyriverbend

Plan C, leave it up to the developer? I've always used Omniauth to achieve multiple provider auth. Omniauth cleans up the authentication strategies, but does not enforce any particular way of storing the provider data. That way the developer gets to choose what makes sense for their implementation. Personally, I'm going to implement what you described in Plan B, one way or another, because that's how my application has to work. That said, I've also worked on apps that only support Facebook auth, so leaving things the way you have them would be fine for that.

In short, I would prefer to implement the provider storage myself, in whatever models make sense for the app I'm building.

asoules avatar Oct 23 '14 12:10 asoules

@jonlemmon - I'm having trouble accessing that link. Can you double-check that it's correct?

@asoules - I definitely don't want to do anything that would complicate things for developers. I'm curious - do you think this gem is doing anything now that makes it difficult to implement multiple provider auth?

@Urbanviking - I'm not sure if I understand what you're suggesting in your second comment. Can you please clarify?

lynndylanhurley avatar Oct 27 '14 01:10 lynndylanhurley

@lynndylanhurley perhaps I'm mistaken, but it seems to make the assumption that a single provider was used to sign up, and the details are stored in User#provider and User#uid. I tried to leave these fields out, but I hit validation errors when devise_token_auth tried to validate that no other users existed with the same provider and uid. The OmniauthCallbacksController also assumes that User#provider and uid will be used. This makes it difficult to connect with multiple providers out of the box.

asoules avatar Oct 27 '14 12:10 asoules

@lynndylanhurley

I removed my previous comment to make myself more clear.

I suggest staying with the single auth provider concept - email or OAuth provider - and remove the User Info attributes including the email attribute.

It would make Devise-Token-Auth simple to use (and understand) for beginners and more experience can override the default controllers and create additional tables to add multiple auth providers and user info as needed.

It seems messy to have some user info attributes in user (authentication) table and particularly the email attribute when email address is also stored in the uid attribute.

ghost avatar Oct 27 '14 15:10 ghost

@Urbanviking - thanks again for your comments.

It would make Devise-Token-Auth simple to use (and understand) for beginners and more experience can override the default controllers and create additional tables to add multiple auth providers and user info as needed.

I expect to be able to perform these basic operations without any trouble:

  • show a user's name when they sign in.
  • display a user's nickname + avatar image next to their user activity (comments, etc).
  • display a list of all users with their names + email addresses (for admin moderation)

With the current implementation, these operations are trivial. But if the name, nickname, email and image fields are removed, then these features will need to be added manually. I think that removing these fields will make the use of this gem more complicated for most developers.

Also, you can override the default implementation as you like.

It seems messy to have some user info attributes in user (authentication) table and particularly the email attribute when email address is also stored in the uid attribute.

"Email" is just another provider, where the unique id (uid) is the email address itself. It would be messier (in my opinion) to authenticate against one field for some users, and another field for other users.

lynndylanhurley avatar Oct 27 '14 20:10 lynndylanhurley

@asoules - I may have a proposal to solve that issue, but I'll need time to work it out. I'll post back ASAP.

lynndylanhurley avatar Oct 27 '14 20:10 lynndylanhurley

Apologies, I offended you with the "messy", but it is not clear that UID column can be an email address when you also have Email column.

The UID and PROVIDER need to form an unique key together. All User Info columns should be optional including EMAIL.

Don't you need a SECRET column on user table for e.g. Facebook authentication?

ghost avatar Oct 27 '14 21:10 ghost

Apologies, I offended you with the "messy"

@Urbanviking - no offense taken! I appreciate your feedback :smiley:

it is not clear that UID column can be an email address when you also have Email column.

uid stands for "unique id". It's the field that is used to identify users (along with the provider field). When users register using email, their email address is their unique id. I don't understand why this would be a problem.

The UID and PROVIDER need to form an unique key together. All User Info columns should be optional including EMAIL.

That is how it works currently.

Don't you need a SECRET column on user table for e.g. Facebook authentication?

The secret is defined in the provider's initializer. There are instructions here.

lynndylanhurley avatar Oct 27 '14 21:10 lynndylanhurley

@lynndylanhurley Only UID is a unique key add_index :users, :uid, unique: true I am suggesting add_index :users, [:provider, :uid], unique: true

If EMAIL is not used for authentication, perhaps it should be listed under User Info instead of Database Authentication in devise_token_auth_create_users.rb migration file :-)

ghost avatar Oct 27 '14 21:10 ghost

@Urbanviking - that makes sense. Can you please post that as a new issue? I'd like to steer this issue back to multi-user authentication if possible :jack_o_lantern:

lynndylanhurley avatar Oct 27 '14 21:10 lynndylanhurley

@lynndylanhurley Was there any progress made with this? :)

ACPK avatar Jan 12 '15 21:01 ACPK

fwiw, I've just implemented my own single-account login using any account (email+password, facebook and Google). The single account ID is the email.

Here's the changes I had to make in order to get this to work, mostly overriding an action in a controller (from devise_token_auth) just to drop some hard-coded things like the AND provider='email' clause in session controller to allow non-email provider to login using email+password. And dropping a check for provider=='email' in PasswordsController#update to allow non-email provider to update and set their passwords.

Here's the change in my project. https://github.com/manshar/manshar/pull/177/files

mkhatib avatar Feb 25 '15 09:02 mkhatib

I agree with @asoules I am trying to implement multiple providers in reference to https://github.com/intridea/omniauth/wiki/Managing-Multiple-Providers (I think this is plan B @lynndylanhurley ) but I have to override so many controllers and concern (anything which assumes user has provider and uid). I would really appreciate it if the gem is flexible enough to allow multiple providers.

phs1919 avatar Mar 04 '15 06:03 phs1919

+1 @phs1919 @mkhatib

harmoney-justins avatar Jul 14 '15 21:07 harmoney-justins

@mkhatib One problem I see with using email as the unique account ID is that users might not use the same email address across their different OAuth provider accounts. Some providers also don't provide email (Twitter for example). Your solution appears to work, but to me, it doesn't seem nearly as robust as Plan B suggested by @lynndylanhurley.

@phs1919, were you able to successfully implement that? I'm planning on doing something similar, but I'm a bit wary of overriding so much code.

frankjwu avatar Jul 14 '15 21:07 frankjwu

@frankjwu i've done something similar with https://github.com/intridea/omniauth/wiki/Managing-Multiple-Providers

But i agree plan B is more robust, allow users to associate with a primary account

harmoney-justins avatar Jul 14 '15 21:07 harmoney-justins

@harmoney-justins What do you use for a User's UID and provider? (assuming you now store that information in the Identities table)

frankjwu avatar Jul 14 '15 22:07 frankjwu

@frankjwu https://gist.github.com/justinsoong/b273626fb8f994dfade3

justinsoong avatar Jul 14 '15 22:07 justinsoong

@justinsoong I might be missing something, but that looks like standard Devise and not DeviseTokenAuth?

The problem I was referring to earlier is that DeviseTokenAuth uses the uid value in the header to lookup and validate tokens (with users identified by a unique uid and provider pair). So if that information is instead now stored in the Identities table, you'll have to go in and make changes to any code that assumes that the User model has uid and provider.

frankjwu avatar Jul 14 '15 23:07 frankjwu

If anyone comes across this issue, would be good to get feedback on PR #453 to see if that would solve your problems.

seddy avatar Apr 14 '16 08:04 seddy

I made some quick changes on https://github.com/wangzuo/devise_token_auth for multiple provider support

wangzuo avatar Mar 26 '21 08:03 wangzuo