Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infinite loop with devise timeoutable + rememberable when session expires - SystemStackError (stack level too deep) #678

Open
ndbroadbent opened this issue Jul 4, 2023 · 1 comment

Comments

@ndbroadbent
Copy link

ndbroadbent commented Jul 4, 2023

Hello, I'm not sure if I should post this here, or on the devise/warden repos. But I'm getting an infinite loop when I use the audited gem with devise "timeoutable". Here is the stack trace I'm seeing:

https://gist.github.com/ndbroadbent/4fd5114ab14d1d3593ea5cb9593ecfc2

I have a custom #current_user method in application_controller.rb that is basically the same as the default Devise one, but adds eager loading for User roles (from the rolify gem.)

  def current_user
    return @current_user if @current_user

    @current_user = warden.authenticate(scope: :user)
    ActiveRecord::Associations::Preloader.new.preload(@current_user, :roles)
    @current_user
  end

I'm not sure why, but this is only crashing with a SystemStackError on production, and it's not crashing locally or on CI. It might be because I have set ulimit to "unlimited" locally and on CI containers, and maybe devise or warden has an internal counter somewhere to detect and prevent infinite loops. I noticed that the current_user method is called around 97 times before it stops trying and moves on, so that's why I think there might be some infinite loop detection happening somewhere. But unfortunately it's not enough to prevent a SystemStackError crash on production.

I'm trying to understand the stack trace, and here's what I think is happening:

  • application calls ApplicationController#current_user
  • This calls warden.authenticate(scope: :user)
  • Devise checks if the session has timed out after inactivity
  • If timed out, it signs out the user automatically. Devise "rememberable" calls the #forget_me! callback to clear remember_token and remember_created_at on the User:
      def forget_me!
        return unless persisted?
        self.remember_token = nil if respond_to?(:remember_token)
        self.remember_created_at = nil if self.class.expire_all_remember_me_on_sign_out
        save(validate: false)
      end
  • The audited gem calls #audit_update for the User update, and then it calls #set_audit_user to set the current user
  • This calls ApplicationController#current_user. @current_user is nil because it has been signed out.
  • Go back to step 1

(I did notice that my Audited db table was taking a ton of space, so this might be related, since I think it's creating audited records in an infinite loop until it crashes. I might have to check this and delete a bunch of rows.)

Solutions:

  • Don't created audited records when updating remember_token and remember_created_at attributes (and possibly other devise attributes.) Maybe set this by default.
  • Audited could somehow detect if the user is being automatically signed out, maybe via a deeper devise/warden integration. Or even just add it's own infinite loop detection internally
  • I'm also going to add test expectations to my own codebase to make sure that the Audited gem never creates too many audit records

Please let me know if you have any other suggestions or feedback. Thanks!

@bkd
Copy link

bkd commented Aug 2, 2023

Yup i have the same issue also

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants