search

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.55k stars 1.14k forks source link

Unable to destroy current user #1220

Open sylvainbx opened 6 years ago

sylvainbx commented 6 years ago

Can anyone tell me why does devise_token_auth is reloading the current user, while I've just deleted it? Obvisouly it ends in an #<ActiveRecord::RecordNotFound: Couldn't find User with 'id'=772> exception, but I can't get how to overpass this error. Is this a bug in devise_token_auth?

The error is raised by devise_token_auth-1.0.0.rc2/app/controllers/devise_token_auth/concerns/set_user_by_token.rb:L119 at @resource.reload

Here's the code snippet of my API::UsersController:

  def destroy
    @user = User.find(params[:id])
    authorize @user
    sign_out @user # I've tried to add this logout before destroying but it changes nothing
    @user.destroy
    head :no_content
  end
MaicolBen commented 6 years ago

@sylvainbx are you sure this happen with 0.1.37 ? I will fix it but I have to know that

sylvainbx commented 6 years ago

Absolutly, that's why I upgraded to 1.0.0.rc2 ... thinking it may have been solved meanwhile

MaicolBen commented 6 years ago

but what is the backtrace in 0.1.37? because the reload is new in 1.0.0.rc2

sylvainbx commented 6 years ago

You're right: the error doesn't came from the same line in 0.1.37, it's thrown by set_user_by_token.rb#L86.

Here's the full stacktrace:

["/rvm.gems.path/activerecord-4.2.0/lib/active_record/relation/finder_methods.rb:336:in `raise_record_not_found_exception!'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/relation/finder_methods.rb:456:in `find_one'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/relation/finder_methods.rb:435:in `find_with_ids'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/relation/finder_methods.rb:71:in `find'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/persistence.rb:411:in `block in reload'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/scoping/default.rb:33:in `block in unscoped'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/relation.rb:302:in `scoping'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/scoping/default.rb:33:in `unscoped'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/persistence.rb:411:in `reload'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/attribute_methods/dirty.rb:36:in `reload'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/autosave_association.rb:219:in `reload'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/locking/pessimistic.rb:62:in `lock!'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/locking/pessimistic.rb:71:in `block in with_lock'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `block in transaction'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/connection_adapters/abstract/transaction.rb:188:in `within_new_transaction'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `transaction'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/transactions.rb:220:in `transaction'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/transactions.rb:277:in `transaction'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/locking/pessimistic.rb:70:in `with_lock'",
 "/rvm.gems.path/devise_token_auth-0.1.37/app/controllers/devise_token_auth/concerns/set_user_by_token.rb:86:in `update_auth_header'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:427:in `block in make_lambda'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:236:in `call'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:236:in `block in halting'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:169:in `call'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:169:in `block in halting'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:151:in `call'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:151:in `block in halting_and_conditional'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:92:in `call'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:92:in `_run_callbacks'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:734:in `_run_process_action_callbacks'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:81:in `run_callbacks'",
 "/rvm.gems.path/actionpack-4.2.0/lib/abstract_controller/callbacks.rb:19:in `process_action'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_controller/metal/rescue.rb:29:in `process_action'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_controller/metal/instrumentation.rb:31:in `block in process_action'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/notifications.rb:164:in `block in instrument'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/notifications/instrumenter.rb:20:in `instrument'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/notifications.rb:164:in `instrument'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_controller/metal/instrumentation.rb:30:in `process_action'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_controller/metal/params_wrapper.rb:250:in `process_action'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/railties/controller_runtime.rb:18:in `process_action'",
 "/rvm.gems.path/actionpack-4.2.0/lib/abstract_controller/base.rb:137:in `process'",
 "/rvm.gems.path/actionview-4.2.0/lib/action_view/rendering.rb:30:in `process'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_controller/metal.rb:195:in `dispatch'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_controller/metal/rack_delegation.rb:13:in `dispatch'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_controller/metal.rb:236:in `block in action'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/routing/route_set.rb:73:in `call'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/routing/route_set.rb:73:in `dispatch'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/routing/route_set.rb:42:in `serve'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/journey/router.rb:43:in `block in serve'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/journey/router.rb:30:in `each'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/journey/router.rb:30:in `serve'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/routing/route_set.rb:802:in `call'",
 "/rvm.gems.path/omniauth-1.3.1/lib/omniauth/strategy.rb:186:in `call!'",
 "/rvm.gems.path/omniauth-1.3.1/lib/omniauth/strategy.rb:164:in `call'",
 "/rvm.gems.path/warden-1.2.7/lib/warden/manager.rb:36:in `block in call'",
 "/rvm.gems.path/warden-1.2.7/lib/warden/manager.rb:35:in `catch'",
 "/rvm.gems.path/warden-1.2.7/lib/warden/manager.rb:35:in `call'",
 "/rvm.gems.path/rack-1.6.4/lib/rack/etag.rb:24:in `call'",
 "/rvm.gems.path/rack-1.6.4/lib/rack/conditionalget.rb:38:in `call'",
 "/rvm.gems.path/rack-1.6.4/lib/rack/head.rb:13:in `call'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/middleware/params_parser.rb:27:in `call'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/middleware/flash.rb:260:in `call'",
 "/rvm.gems.path/rack-1.6.4/lib/rack/session/abstract/id.rb:225:in `context'",
 "/rvm.gems.path/rack-1.6.4/lib/rack/session/abstract/id.rb:220:in `call'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/middleware/cookies.rb:560:in `call'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/query_cache.rb:36:in `call'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/connection_adapters/abstract/connection_pool.rb:647:in `call'",
 "/rvm.gems.path/activerecord-4.2.0/lib/active_record/migration.rb:378:in `call'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/middleware/callbacks.rb:29:in `block in call'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:88:in `call'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:88:in `_run_callbacks'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:734:in `_run_call_callbacks'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/callbacks.rb:81:in `run_callbacks'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/middleware/callbacks.rb:27:in `call'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/middleware/reloader.rb:73:in `call'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/middleware/remote_ip.rb:78:in `call'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/middleware/debug_exceptions.rb:17:in `call'",
 "/rvm.gems.path/web-console-2.2.1/lib/web_console/middleware.rb:39:in `call'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/middleware/show_exceptions.rb:30:in `call'",
 "/rvm.gems.path/railties-4.2.0/lib/rails/rack/logger.rb:38:in `call_app'",
 "/rvm.gems.path/railties-4.2.0/lib/rails/rack/logger.rb:20:in `block in call'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/tagged_logging.rb:68:in `block in tagged'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/tagged_logging.rb:26:in `tagged'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/tagged_logging.rb:68:in `tagged'",
 "/rvm.gems.path/railties-4.2.0/lib/rails/rack/logger.rb:20:in `call'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/middleware/request_id.rb:21:in `call'",
 "/rvm.gems.path/rack-1.6.4/lib/rack/methodoverride.rb:22:in `call'",
 "/rvm.gems.path/rack-1.6.4/lib/rack/runtime.rb:18:in `call'",
 "/rvm.gems.path/activesupport-4.2.0/lib/active_support/cache/strategy/local_cache_middleware.rb:28:in `call'",
 "/rvm.gems.path/rack-1.6.4/lib/rack/lock.rb:17:in `call'",
 "/rvm.gems.path/actionpack-4.2.0/lib/action_dispatch/middleware/static.rb:113:in `call'",
 "/rvm.gems.path/rack-1.6.4/lib/rack/sendfile.rb:113:in `call'",
 "/rvm.gems.path/railties-4.2.0/lib/rails/engine.rb:518:in `call'",
 "/rvm.gems.path/railties-4.2.0/lib/rails/application.rb:164:in `call'",
 "/rvm.gems.path/rack-1.6.4/lib/rack/content_length.rb:15:in `call'",
 "/rvm.gems.path/puma-2.15.3/lib/puma/server.rb:541:in `handle_request'",
 "/rvm.gems.path/puma-2.15.3/lib/puma/server.rb:388:in `process_client'",
 "/rvm.gems.path/puma-2.15.3/lib/puma/server.rb:270:in `block in run'",
 "/rvm.gems.path/puma-2.15.3/lib/puma/thread_pool.rb:106:in `call'",
 "/rvm.gems.path/puma-2.15.3/lib/puma/thread_pool.rb:106:in `block in spawn_thread'"]
MaicolBen commented 6 years ago

@sylvainbx This isn't a bug itself, because you should have skipped the callback like the registrations controllers's destroy action, but I made a PR preventing both exceptions

sylvainbx commented 6 years ago

Thanks for your fix @MaicolBen. But strangely it doesn't solve my issue... I've made some tests and @resource.persisted? is returning true even if @resource.reload throws #<ActiveRecord::RecordNotFound: Couldn't find User with 'id'=772>. What do you mean by skipping the callbacks?

MaicolBen commented 6 years ago

@sylvainbx Oh that's a pity, maybe we should use destroyed? instead. For the callbacks, check https://github.com/lynndylanhurley/devise_token_auth/blob/master/app/controllers/devise_token_auth/registrations_controller.rb#L8.

sylvainbx commented 6 years ago

Unfortunately @resource.destroyed? => false. I don't understand why these properties are not updated... Anyway, the only way I found to overpass the issue was to change the code this way:

// ...
else
  begin
    unless @resource.reload.valid?
      @resource = resource_class.find(@resource.to_param) # errors remain after reload
      # if we left the model in a bad state, something is wrong in our app
      unless @resource.valid?
        raise DeviseTokenAuth::Errors::InvalidModel, "Cannot set auth token in invalid model. Errors: #{@resource.errors.full_messages}"
      end
    end
  rescue ActiveRecord::RecordNotFound
    return
  end
  refresh_headers

But I'm not really sure if this is a relevant way to proceed ... Maybe we should rather investigate on destroyed? or persisted??

Meanwhile, I've tried to add skip_after_action :update_auth_header, only: [:destroy] at the top of my controller, as you suggested, and the issue disappears. Maybe documenting this point can be an sufficient fix?

MaicolBen commented 6 years ago

The first solution is awful, and the second one is actually what the current code uses, I don't know where we can document this, maybe this issue is enough, thank you for the update!

sylvainbx commented 6 years ago

Maybe in the docs ? :)