lynndylanhurley / lynndylanhurley/devise_token_auth
dta_find_by on wrong model with set_user_by_token
Nobody has claimed this yet.
- Dominant language
- Ruby
- Stars
- 3.6k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
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
```user.rb
class User < ApplicationRecord
# Include default devise modules. Others available are:
# :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
devise :database_authenticatable, :registerable, :validatable
end
```
```admin.rb
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
```routes.rb
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
```appication_controller.rb
class ApplicationController < ActionController::Base
before_action :check_signed
private
def check_signed
if user_signed_in?
puts "User signed in!!!"
end
end
end
```
```api/auth/sessions_controller.rb
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`?
```set_user_by_token.rb
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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with app/controllers/devise_token_auth/concerns/set_user_by_token.rb at the dta_find_by call, then inspect resource_finder.rb and the related Devise mappings. Reproduce the Admin and User setup from the issue and determine an agreed behavior that avoids the NoMethodError without weakening authentication; done requires a reviewed fix and regression coverage for the mixed mappings.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rails, ruby
- Domain
- api, authentication
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100