Multi auth methods per resource

README.md

@@ -52,6 +52,7 @@ Please read the [issue reporting guidelines](#issue-reporting) before posting is
 * [Usage TL;DR](#usage-tldr)
 * [Configuration Continued](#configuration-cont)
   * [Initializer Settings](#initializer-settings)
+  * [Single vs Multiple authentication methods](#single-vs-multiple-authentication-methods-per-resource)
   * [OmniAuth Authentication](#omniauth-authentication)
   * [OmniAuth Provider Settings](#omniauth-provider-settings)
   * [Email Authentication](#email-authentication)
@@ -126,7 +127,7 @@ The following events will take place when using the install generator:
 
 * A concern will be included by your application controller at `app/controllers/application_controller.rb`. [Read more](#controller-methods).
 
-* A migration file will be created in the `db/migrate` directory. Inspect the migrations file, add additional columns if necessary, and then run the migration:
+* A migration file will be created in the `db/migrate` directory. Inspect the migrations file, add or remove columns if necessary, and then run the migration:
 
   ~~~bash
   rake db:migrate
@@ -135,6 +136,7 @@ The following events will take place when using the install generator:
 You may also need to configure the following items:
 
 * **OmniAuth providers** when using 3rd party oauth2 authentication. [Read more](#omniauth-authentication).
+* **Allow multiple authentication methods** for resource. [Read more](#single-vs-multiple-authentication-methods-per-resource)
 * **Cross Origin Request Settings** when using cross-domain clients. [Read more](#cors).
 * **Email** when using email registration. [Read more](#email-authentication).
 * **Multiple model support** may require additional steps. [Read more](#using-multiple-models).
@@ -196,6 +198,82 @@ Devise.setup do |config|
 end
 ~~~
 
+## Single vs Multiple authentication methods per resource
+
+By default, `devise_token_auth` only allows a single authentication per resource.
+
+What does this mean? Let's take the example of having a Customer model and you want to let people sign up with Facebook or with their email address. If they register with their Facebook account, then you'll have one row in your `customers` table, and if they then register with their email address, you'll have **another** row in your `customers` table. Both for the same real life person.
+
+This is because multiple sign in methods for a single resource are difficult to maintain and reason about, particularly when trying to build a suitable UX. The only problem is the expectation that users will always use the same authentication method.
+
+BUT, `devise_token_auth` is awesome enough (like `devise`) to let you manage multiple methods on a single resource without sacrificing your data integrity.  Using our previous example, this means you can have a single Customer row which can be authenticated with **either** Facebook **or** their email address.
+
+### Setting up single authentication per resource (default behaviour)
+
+When you run `rails g devise_token_auth:install User auth`, you will have a migration setup which will look something like this:
+
+~~~ruby
+# db/migrate/20151116175322_add_devise_token_auth_fields_to_users.rb
+class AddDeviseTokenAuthFieldsToUsers < ActiveRecord::Migration
+  t.string :provider, :null => false, :default => "email"
+  t.string :uid, :null => false, :default => ""
+  ...
+end
+~~~
+
+The `provider` and `uid` fields are used to record what method and what identifier we will use for identifying and authing a `User`. For example:
+
+| Signup method | provider | uid |
+|---|---|---|
+| email: [email protected] | email | [email protected] |
+| facebook user id: 12345 | facebook | 12345 |
+
+And that's pretty much all you have to do!
+
+**The good thing** about this method is that it's simplest to implement from a UX point of view and, consequently, the most common implementation you'll see at the moment.
+
+**The problem** is that you may end up with a single person creating multiple accounts when they don't mean to because they've forgotten how they originally authenticated. In order to make this happen, the gem has to be fairly opinionated about how to manage your domain objects (e.g. it allows multiple users with the same "email" field)
+
+### Setting up multiple authentication methods per resource
+
+You may want to let a user log in with multiple methods to the same account. In order to do this, the `devise_token_auth` gem is unopinionated on how you've built your model layer, and just requires that you declare how to look up various resources.
+
+If using this methodology, you **do not need provider/uid columns on your resource table**, so you can remove these from the generated migration when running `rails g devise_token_auth:install`.
+
+Instead, you need to register finder methods defining how to get to your resource from a particular provider. If you don't register one, it falls back to the default behaviour for single authentication of querying provider/uid (if those columns exist).
+
+An example of registering these finders is done as follows:
+
+~~~ruby
+class User < ActiveRecord::Base
+  # In this example, the twitter id is simply stored directly on the User
+  resource_finder_for :twitter,  ->(twitter_id)  { find_by(twitter_id: twitter_id) }
+
+  # In this example, the external facebook user is modelled seperately from the
+  # User, and we need to go through an association to find the User to
+  # authenticate against
+  resource_finder_for :facebook, ->(facebook_id) { FacebookUser.find_by(facebook_id: facebook_id).user }
+end
+~~~
+
+You'll need to register a finder for each authentication method you want to allow users to have. Given a specific `uid` (for omniauth, this will most likely be the foreign key onto the third party object). You can register a `Proc` or a `Lambda` for this, and each time we get a request which has been authed in this manner, we will look up using it.
+
+**WARNING**: Bear in mind that these finder methods will get called on every authenticated request. So consider performance carefully. For example, with the `:facebook` finder above, we may want to add an `.includes(:user)` to keep the number of DB queries down.
+
+#### Default finders when using multiple authentication
+
+You don't need to define a `resource_finder_for` callback for something registered as a `Devise.authentication_key` (e.g. `:email` or `:username`, see the [Devise wiki](https://github.com/plataformatec/devise/wiki/How-To:-Allow-users-to-sign-in-using-their-username-or-email-address#user-content-tell-devise-to-use-login-in-the-authentication_keys)), then we will call a `find_by` using that column. Consequently:
+
+~~~ruby
+class Users < ActiveRecord::Base
+  # We are allowing users to authenticating with either their email or username
+  devise :database_authenticatable, authentication_keys: [:username, :email]
+
+  # Therefore, we don't need the following:
+  # resource_finder_for :username, ->(username) { find_by(username: username) }
+end
+~~~
+
 ## OmniAuth authentication
 
 If you wish to use omniauth authentication, add all of your desired authentication provider gems to your `Gemfile`.
@@ -211,6 +289,8 @@ Then run `bundle install`.
 
 [List of oauth2 providers](https://github.com/intridea/omniauth/wiki/List-of-Strategies)
 
+Consider whether you want to allow [single or multiple](#single-vs-multiple-authentication-methods-per-resource) authentication methods per resource.
+
 ## OmniAuth provider settings
 
 In `config/initializers/omniauth.rb`, add the settings for each of your providers.
@@ -438,7 +518,7 @@ The authentication information should be included by the client in the headers o
 "token-type":   "Bearer",
 "client":       "xxxxx",
 "expiry":       "yyyyy",
-"uid":          "zzzzz"
+"uid":          "zzzzz provider"
 ~~~
 
 The authentication headers (each one is a seperate header) consists of the following params:
@@ -448,7 +528,7 @@ The authentication headers (each one is a seperate header) consists of the follo
 | **`access-token`** | This serves as the user's password for each request. A hashed version of this value is stored in the database for later comparison. This value should be changed on each request. |
 | **`client`** | This enables the use of multiple simultaneous sessions on different clients. (For example, a user may want to be authenticated on both their phone and their laptop at the same time.) |
 | **`expiry`** | The date at which the current session will expire. This can be used by clients to invalidate expired tokens without the need for an API request. |
-| **`uid`** | A unique value that is used to identify the user. This is necessary because searching the DB for users by their access token will make the API susceptible to [timing attacks](http://codahale.com/a-lesson-in-timing-attacks/). |
+| **`uid`** | A unique value that is used to identify the user, concatenated with the provider the identifier is for (e.g. `12345 facebook` or `[email protected] email`). This is necessary because searching the DB for users by their access token will make the API susceptible to [timing attacks](http://codahale.com/a-lesson-in-timing-attacks/). |
 
 The authentication headers required for each request will be available in the response from the previous request. If you are using the [ng-token-auth](https://github.com/lynndylanhurley/ng-token-auth) AngularJS module or the [jToker](https://github.com/lynndylanhurley/j-toker) jQuery plugin, this functionality is already provided.
 

app/controllers/devise_token_auth/concerns/set_user_by_token.rb

@@ -49,6 +49,8 @@ def set_user_by_token(mapping=nil)
       if devise_warden_user && devise_warden_user.tokens[@client_id].nil?
         @used_auth_by_token = false
         @resource = devise_warden_user
+        # REVIEW: Why are we bothering to create an auth token here? It won't
+        # get used anywhere by the looks of it...?
         @resource.create_new_auth_token
       end
     end
@@ -64,17 +66,22 @@ def set_user_by_token(mapping=nil)
 
     return false unless @token
 
-    # mitigate timing attacks by finding by uid instead of auth token
-    user = uid && rc.find_by(uid: uid)
+    # NOTE: By searching for the user by an identifier instead of by token, we
+    # mitigate timing attacks
+    #
+    @provider_id, @provider = uid.split # e.g. ["12345", "facebook"] or ["[email protected]", "email"]
+    resource = rc.find_resource(@provider_id, @provider)
 
-    if user && user.valid_token?(@token, @client_id)
+    if resource && resource.valid_token?(@token, @client_id)
+      # REVIEW: why is this looking at :user? Shouldn't it be mapping to handle
+      # multiple devise models such as Admin?
       # sign_in with bypass: true will be deprecated in the next version of Devise
       if self.respond_to? :bypass_sign_in
-        bypass_sign_in(user, scope: :user)
+        bypass_sign_in(resource, scope: rc.to_s.downcase.to_sym)
       else
-        sign_in(:user, user, store: false, bypass: true)
+        sign_in(rc.to_s.downcase.to_sym, resource, store: false, bypass: true)
       end
-      return @resource = user
+      return @resource = resource
     else
       # zero all values previously set values
       @client_id = nil
@@ -94,8 +101,7 @@ def update_auth_header
       # cleared by sign out in the meantime
       return if @resource.reload.tokens[@client_id].nil?
 
-      auth_header = @resource.build_auth_header(@token, @client_id)
-
+      auth_header = @resource.build_auth_header(@token, @client_id, @provider_id, @provider)
       # update the response header
       response.headers.merge!(auth_header)
 
@@ -117,7 +123,7 @@ def update_auth_header
         # extend expiration of batch buffer to account for the duration of
         # this request
         if @is_batch_request
-          auth_header = @resource.extend_batch_buffer(@token, @client_id)
+          auth_header = @resource.extend_batch_buffer(@token, @client_id, @provider_id, @provider)
 
           # Do not return token for batch requests to avoid invalidated
           # tokens returned to the client in case of race conditions.
@@ -129,7 +135,10 @@ def update_auth_header
 
         # update Authorization response header with new token
         else
-          auth_header = @resource.create_new_auth_token(@client_id)
+          auth_header = @resource.create_new_auth_token(@client_id, @provider_id, @provider)
+
+          # update the response header
+          response.headers.merge!(auth_header)
         end
 
         # update the response header

app/controllers/devise_token_auth/confirmations_controller.rb

@@ -5,6 +5,8 @@ def show
 
       if @resource && @resource.id
         # create client id
+        #
+        # REVIEW: Why isn't this using resource_class.create_new_auth_token?
         client_id  = SecureRandom.urlsafe_base64(nil, false)
         token      = SecureRandom.urlsafe_base64(nil, false)
         token_hash = BCrypt::Password.create(token)

app/controllers/devise_token_auth/omniauth_callbacks_controller.rb

@@ -27,15 +27,14 @@ def redirect_callbacks
 
     def omniauth_success
       get_resource_from_auth_hash
-      create_token_info
-      set_token_on_resource
-      create_auth_params
+      @auth_params = create_token_info
 
       if resource_class.devise_modules.include?(:confirmable)
         # don't send confirmation email!!!
         @resource.skip_confirmation!
       end
 
+      # REVIEW: Shouldn't this be 'devise_mapping' instead of :user?
       sign_in(:user, @resource, store: false, bypass: false)
 
       @resource.save!
@@ -157,30 +156,33 @@ def set_random_password
     end
 
     def create_token_info
-      # create token info
-      @client_id = SecureRandom.urlsafe_base64(nil, false)
-      @token     = SecureRandom.urlsafe_base64(nil, false)
-      @expiry    = (Time.now + @resource.token_lifespan).to_i
+      # These need to be instance variables so that we set the auth header info
+      # correctly
+      @provider_id = auth_hash['uid']
+      @provider = auth_hash['provider']
+
+      auth_values = @resource.create_new_auth_token(nil, @provider_id, @provider).symbolize_keys
+      @client_id = auth_values['client']
+      @token     = auth_values['access-token']
+      @expiry    = auth_values['expiry']
       @config    = omniauth_params['config_name']
-    end
-
-    def create_auth_params
-      @auth_params = {
-        auth_token:     @token,
-        client_id: @client_id,
-        uid:       @resource.uid,
-        expiry:    @expiry,
-        config:    @config
-      }
-      @auth_params.merge!(oauth_registration: true) if @oauth_registration
-      @auth_params
-    end
 
-    def set_token_on_resource
-      @resource.tokens[@client_id] = {
-        token: BCrypt::Password.create(@token),
-        expiry: @expiry
-      }
+      # The #create_new_auth_token values returned here have the token set as
+      # the "access-token" value. Unfortunately, the previous implementation
+      # would render this attribute out as "auth_token". Which is inconsistent
+      # and wrong, but if people are using the body of the auth response
+      # instead of the headers, they may see failures here. Not changing at the
+      # moment as this would therefore be a breaking change. Same goes for
+      # client_id/client.
+      #
+      # TODO: Fix this so that it consistently returns this in an
+      # "access-token" field instead of an "auth_token".
+      auth_values[:auth_token] = auth_values.delete(:"access-token")
+      auth_values[:client_id] = auth_values.delete(:client)
+
+      auth_values.merge!(config: @config)
+      auth_values.merge!(oauth_registration: true) if @oauth_registration
+      auth_values
     end
 
     def render_data(message, data)
@@ -229,13 +231,15 @@ def fallback_render(text)
     end
 
     def get_resource_from_auth_hash
-      # find or create user by provider and provider uid
-      @resource = resource_class.where({
-        uid:      auth_hash['uid'],
-        provider: auth_hash['provider']
-      }).first_or_initialize
-
-      if @resource.new_record?
+      @resource = resource_class.find_resource(
+        auth_hash['uid'],
+        auth_hash['provider']
+      )
+
+      if @resource.nil?
+        @resource          = resource_class.new
+        @resource.uid      = auth_hash['uid']      if @resource.has_attribute?(:uid)
+        @resource.provider = auth_hash['provider'] if @resource.has_attribute?(:provider)
         @oauth_registration = true
         set_random_password
       end

app/controllers/devise_token_auth/passwords_controller.rb

@@ -28,8 +28,10 @@ def create
       end
 
       @email = get_case_insensitive_field_from_resource_params(:email)
-      @resource = find_resource(:uid, @email)
 
+      field = resource_class.authentication_field_for(resource_params.keys.map(&:to_sym))
+
+      @resource = resource_class.find_resource(resource_params[field], field) if field
       @errors = nil
       @error_status = 400
 
@@ -48,7 +50,10 @@ def create
           @errors = @resource.errors
         end
       else
-        @errors = [I18n.t("devise_token_auth.passwords.user_not_found", email: @email)]
+        # TODO: The resource_params could be a "username" field depending on
+        # what keys the resource uses for authentication. This translation
+        # should be updated to reflect this.
+        @errors = [I18n.t("devise_token_auth.passwords.user_not_found", email: resource_params[field])]
         @error_status = 404
       end
 

app/controllers/devise_token_auth/registrations_controller.rb

@@ -54,6 +54,8 @@ def create
 
           else
             # email auth has been bypassed, authenticate user
+            #
+            # REVIEW: Shouldn't this be calling resource_class.create_new_auth_token?
             @client_id = SecureRandom.urlsafe_base64(nil, false)
             @token     = SecureRandom.urlsafe_base64(nil, false)
 

app/controllers/devise_token_auth/sessions_controller.rb

@@ -9,14 +9,12 @@ def new
     end
 
     def create
-      # Check
-      field = (resource_params.keys.map(&:to_sym) & resource_class.authentication_keys).first
+      field = resource_class.authentication_field_for(resource_params.keys.map(&:to_sym))
 
-      @resource = nil
       if field
         q_value = get_case_insensitive_field_from_resource_params(field)
 
-        @resource = find_resource(field, q_value)
+        @resource = resource_class.find_resource(resource_params[field], field)
       end
 
       if @resource && valid_params?(field, q_value) && ([email protected]_to?(:active_for_authentication?) || @resource.active_for_authentication?)
@@ -26,15 +24,17 @@ def create
           return
         end
         # create client id
-        @client_id = SecureRandom.urlsafe_base64(nil, false)
-        @token     = SecureRandom.urlsafe_base64(nil, false)
+        auth_values = @resource.create_new_auth_token(nil, resource_params[field], field)
 
-        @resource.tokens[@client_id] = {
-          token: BCrypt::Password.create(@token),
-          expiry: (Time.now + @resource.token_lifespan).to_i
-        }
-        @resource.save
+        # These instance variables are required when updating the auth headers
+        # at the end of the request, see:
+        #   DeviseTokenAuth::Concerns::SetUserByToken#update_auth_header
+        @token       = auth_values["access-token"]
+        @client_id   = auth_values["client"]
+        @provider    = "email"
+        @provider_id = @resource.email
 
+        # REVIEW: Shouldn't this be a "mapping" option, rather than a :user?
         sign_in(:user, @resource, store: false, bypass: false)
 
         yield @resource if block_given?