Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dta_find_by on wrong model with set_user_by_token #1629

Open
stimlocy opened this issue Jun 3, 2024 · 0 comments
Open

dta_find_by on wrong model with set_user_by_token #1629

stimlocy opened this issue Jun 3, 2024 · 0 comments

Comments

@stimlocy
Copy link

stimlocy commented Jun 3, 2024

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).

user = (uid && rc.dta_find_by(uid: uid)) || (other_uid && rc.dta_find_by("#{DeviseTokenAuth.other_uid}": other_uid))

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant