sorcery icon indicating copy to clipboard operation
sorcery copied to clipboard

API controller breaks when using activity_logging submodule

Open matssigge opened this issue 5 years ago • 3 comments

Hi!

I'm having a problem upgrading Sorcery beyond 0.12 (yes, I'm behind). We have a couple of API controllers, and calls to them break when using Sorcery 0.13 or 0.14. The error I get is

undefined local variable or method `cookies',

and the stacktrace points to the ActivityLogging submodule, and the method register_last_activity_time_to_db. It looks like all submodules are being included in all controllers (both API and "normal" ones), but ActivityLogging doesn't work (in its current form) with APIs.

Modifying the self.included method of the ActivityLogging module to check if the controller inherits from ActionController::API solves the problem for me. I would be happy to provide a pull request, but I'm not sure this is the best way to solve the problem. Input would be appreciated.

Configuration

  • Sorcery Version: 0.14
  • Ruby Version: 2.6.4
  • Framework: Rails 5.2.3
  • Platform: macOS

Steps to Reproduce

I created a small Rails app where it's easy to reproduce. It's at https://github.com/matssigge/sorcery_test. Just clone, run the rails app, and then curl localhost:3000/api/test.

matssigge avatar Sep 02 '19 15:09 matssigge

Your controller extends ActionController::API, which does not include ActionController::Cookies module. Since you have remember_me submodule activated, it's trying to log user in from cookie.

To have access to cookies, you have to include ActionController::Cookies in your API controller:

class MyApiController < ActionController::API
  include ActionController::Cookies
  ...
end

If you are running rails in API-only mode, you also have to include cookies middleware:

# config/application.rb
config.middleware.use ActionDispatch::Cookies

Here's the relevant call stack for future reference:

NameError (undefined local variable or method `cookies' for #<MyApiController:0x00007f9f097ef248>):

sorcery (0.14.0) lib/sorcery/controller/submodules/remember_me.rb:62:in `login_from_cookie'
sorcery (0.14.0) lib/sorcery/controller.rb:131:in `block in login_from_other_sources'
sorcery (0.14.0) lib/sorcery/controller.rb:130:in `each'
sorcery (0.14.0) lib/sorcery/controller.rb:130:in `find'
sorcery (0.14.0) lib/sorcery/controller.rb:130:in `login_from_other_sources'
sorcery (0.14.0) lib/sorcery/controller.rb:86:in `current_user'
sorcery (0.14.0) lib/sorcery/controller.rb:79:in `logged_in?'
sorcery (0.14.0) lib/sorcery/controller/submodules/activity_logging.rb:69:in `register_last_activity_time_to_db'
activesupport (5.2.3) lib/active_support/callbacks.rb:426:in `block in make_lambda'
activesupport (5.2.3) lib/active_support/callbacks.rb:247:in `block in halting'
activesupport (5.2.3) lib/active_support/callbacks.rb:517:in `block in invoke_after'
activesupport (5.2.3) lib/active_support/callbacks.rb:517:in `each'
activesupport (5.2.3) lib/active_support/callbacks.rb:517:in `invoke_after'
activesupport (5.2.3) lib/active_support/callbacks.rb:133:in `run_callbacks'
actionpack (5.2.3) lib/abstract_controller/callbacks.rb:41:in `process_action'

As for the sorcery, better approach would be to fail more gracefully and simple fail the auto-login without raising errors.

mladenilic avatar Oct 28 '19 23:10 mladenilic

Sorry for the very late reply.

I'm aware that the direct cause of the problem is not having access to cookies in the API controller, but cookies aren't relevant in an API. So even though the failure occurs in the RememberMe module, the real problem is that ActivityLogging is being triggered in an API controller, where there is not always a clear user (and in general there won't be any cookies set anyway).

Adding the cookies middleware will solve the problem, but it feels like a hack. A better solution would be to not include the ActivityLogging module in API controllers, alternatively providing a way to explicitly identify a user from the API controller (but it needs to handle the case that there's no user, since an API call might not always be associated with one).

matssigge avatar Jan 07 '20 12:01 matssigge

I'm still working on getting the unload_plugin function to actually unload base rails callbacks such as before_action and after_action, but unloading a specific module is now possible, and worst case scenario the base rails callbacks can be skipped in nested controllers by calling skip_before_action / skip_after_action.

joshbuker avatar Jun 04 '21 23:06 joshbuker