Multiple calls to db on authenticate

justinsoong commented 8 years ago

it seems like authenticate_user! is making multiple queries up to 5 times for the current user, seems a little bit un necessary, causing some response time increases for me, will dig though the code base when i wake up in the morning.

but anyone noticing this?

qd3v commented 8 years ago

The problem is even worse. This results I've got on empty db with one created user, which never signed in before. There are two requests to /sign_in shown, done within one browser session (simple button on same page clicked twice) with j-toker lib.

multiple_updates

There are these problems here:

user model (Dj in that case) updated twice (as you've noticed). It's not just updated it creates new token tuple. So you ended up with huge tokens column with useless data...
it seems grape_token_auth does not respect client id (maybe I understand something wrong, but it should stay the same between requests if once set and everything else is valid?):

grape_auth_token_clients

The code is simple, nothing added/removed:

namespace :djs do
  include GrapeTokenAuth::MountHelpers
  mount_registration(to: '/', for: :dj)
  mount_sessions(to: '/', for: :dj)
  mount_token_validation(to: '/', for: :dj)
end

namespace :users do
  include GrapeTokenAuth::MountHelpers
  mount_registration(to: '/', for: :user)
  mount_sessions(to: '/', for: :user)
  mount_token_validation(to: '/', for: :user)
end

I'm at pre-production stage with my project I'm working on, and this issues make me a bit nervous... :( Please let me know, maybe I'm missing something, or it's really a bug? Thanks!

mcordell commented 8 years ago

Thanks for bringing this to my attention and the detailed bug report, I will look into this this week and report back.

qd3v commented 8 years ago

Thanks! To give you the full picture:

grape_token_auth (0.1.0)
grape (0.16.2)
grape-entity (0.5.1)
rails (4.2.6)

We will drop Rails in near future, for now it acts as a root app (and env) w/o any logic, i.e. no devise used or something.

mcordell commented 8 years ago

Update: I've reproduced the issue in the demo app and am working on a fix.

mcordell commented 8 years ago

@vanburg can you check the attached PR. See the note in the PR about the two validation calls.

qd3v commented 8 years ago

@mcordell Opps, commented here: https://github.com/mcordell/grape_token_auth/pull/44#issuecomment-222329716 In short: this works :)

qd3v commented 8 years ago

Found another (I think close) problem. While using OAuth here's what happens:

omni_problem

Strange never-used new token tuple creation appears. This is the object j-toker receives after successful sign-in with instagram:

omni_response

As you can see we use client_id generated once User created.

BTW, while I use in on Rails i needed to fix sign out with this patch: #45 Please, take a look. Thanks!

mcordell / grape_token_auth

Multiple calls to db on authenticate #39