mcordell / grape_token_auth

Token auth for grape apps
MIT License
52 stars 19 forks source link

`case_insensitive_keys` returns nil #37

Closed stephanvane closed 8 years ago

stephanvane commented 8 years ago

When I put

include GrapeTokenAuth::ActiveRecord::TokenAuth

In my User model, the case_insensitive_keys method returns nil (which breaks everything)

Without include GrapeTokenAuth::ActiveRecord::TokenAuth

[1] pry(main)> User.case_insensitive_keys
=> [:email]

With include GrapeTokenAuth::ActiveRecord::TokenAuth

[1] pry(main)> User.case_insensitive_keys
=> nil

Any suggestions?

mcordell commented 8 years ago

This is because this line https://github.com/mcordell/grape_token_auth/blob/master/lib/grape_token_auth/orm_integrations/active_record_token_auth.rb#L62

clobbers Devise's case_insensitive_keys here is a possible fix:

https://github.com/mcordell/grape_token_auth/compare/devise-clobber-fix?expand=1

Let me know if this sorts out the problem for you?

stephanvane commented 8 years ago

Hi, sorry for the late reply. Thanks for having a look at this.

Unfortunately, this fix only works when the include GrapeTokenAuth::ActiveRecord::TokenAuth line is placed below the devise :database_authenticatable,.... line. Device defines the case_insensitive_keys method, so the new unless checks won't have any effect if device is included later on.

At the moment I work around this issue by placing the following line in my user.rb

@case_insensitive_keys = [:email]

But this isn't really a good, long term, solution of course.

evserykh commented 8 years ago
class User < ActiveRecord::Base
  ...
  include GrapeTokenAuth::ActiveRecord::TokenAuth

  devise :database_authenticatable, :registerable, :recoverable, 
         :rememberable, :trackable, :confirmable,
         case_insensitive_keys: [:email] 
end

works for me but it's also not a very good solution I guess

mcordell commented 8 years ago

I just pushed a new fix to the 'devise-clobber-fix' branch, can someone confirm that it works?

mcordell commented 8 years ago

Merged, re-open if this persists