lynndylanhurley / devise_token_auth

Token based authentication for Rails JSON APIs. Designed to work with jToker and ng-token-auth.
Do What The F*ck You Want To Public License
3.54k stars 1.13k forks source link

dta_find_by on wrong model with set_user_by_token #1629

Open stimlocy opened 4 months ago

stimlocy commented 4 months ago

Since the issues #399 and #820 are still seem to be unresolved, I have created a new issue thread. My case would be more like issue #820 (Please forgive the poor English as I am using a translation tool.)

In my app, There are Admin and User. User signin by Devise, Admin signin by DeviseTokenAuth.

If user_signed_in? or current_user is used in the before_action, set_user_by_token method is passed :user, and rc is assigned User. However, since the User is dedicated to Devise signin, DeviseTokenAuth::Concerns::User does not include it. Therefore, it does not have a dta_find_by method, which causes a NoMethodError in rc.dta_find_by(uid: uid).

https://github.com/lynndylanhurley/devise_token_auth/blob/95b7b91beddc6bff09ffc0d4e3a143e8f73cf606/app/controllers/devise_token_auth/concerns/set_user_by_token.rb#L84

versions

ruby 3.3.0 Rails 7.1.3.3

Devise 4.9.4 DeviseTokenAuth 1.2.3

models

class User < ApplicationRecord
  # Include default devise modules. Others available are:
  # :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
  devise :database_authenticatable, :registerable, :validatable
end
class Admin < ActiveRecord::Base
  # Include default devise modules. Others available are:
  # :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
  devise :database_authenticatable, :registerable, :validatable
  include DeviseTokenAuth::Concerns::User
end

routes

Rails.application.routes.draw do
  devise_for :users

  namespace :api, defaults: {format: 'json'} do
    mount_devise_token_auth_for 'Admin', at: 'auth', controllers: {
      sessions: 'api/auth/sessions',
    }
  end
end

signin -> /api/auth/sign_in signout -> /api/auth/sign_out validate -> /auth/validate_token

controllers

class ApplicationController < ActionController::Base
  before_action :check_signed

  private

  def check_signed
    if user_signed_in?
      puts "User signed in!!!"
    end
  end
end
class Api::Auth::SessionsController < DeviseTokenAuth::SessionsController
  skip_before_action :verify_authenticity_token

  protected

  def set_user_by_token(mapping = nil)
    binding.pry
    super(mapping)
  end
end

fix suggestion

I am not familiar with security and DeviseTokenAuth specifications.

How about implementing an return before doing rc.dta_find_by?

unless rc.included_modules.include?(DeviseTokenAuth::Concerns::User)
  return
end

Maybe it takes away the means to notice when a developer forgets that he should include DeviseTokenAuth::Concerns::User

Alternatively, resource_class method in resource_finder.rb without Devise.mappings, change as in DeviseTokeunAuth.mappings Is it possible to set only the list of models needed from mount_devise_token_auth_for ?

I'd like to hear everyone's opinion.