heartcombo / devise

Flexible authentication solution for Rails with Warden.
http://blog.plataformatec.com.br/tag/devise/
MIT License
23.89k stars 5.54k forks source link

New session fails with undefined method `empty?' for #<User #5571

Closed dvodvo closed 1 year ago

dvodvo commented 1 year ago

some context

For deployment reasons, the local machine had to uninstall and re-install ruby 3.2.1 and regenerate Gemfile.lock
Before the change, the following error was never observed.

Rails was declared in Gemfile as gem 'rails', '~> 7.0', '>= 7.0.4'. Given a new release on previous day, the application was bundled with 7.0.4.3. An attempt to rollback by uncommenting previous line and inserting gem 'rails', '7.0.4.2' did not modify the new bahaviour seen.

gem 'devise', '~> 4.8', '>= 4.8.1' but appears in Gemfile.lock as

    devise (4.9.0)
      bcrypt (~> 3.0)
      orm_adapter (~> 0.1)
      railties (>= 4.1.0)
      responders
      warden (~> 1.2.3)

I can confirm that the following behaviour disappears if devise is frozen to 4.8.1 and application re-bundled.

Current behavior

The log file registers with proper params

Started POST "/users/sign_in" for 127.0.0.1 at 2023-03-14 23:15:29 +0100
Processing by Users::SessionsController#create as TURBO_STREAM
 Parameters: {"authenticity_token"=>"[FILTERED]", "user"=>{"login_name"=>"[email address]", "password"=>"[FILTERED]", "remember_me"=>"0"}, "commit"=>"log in"}

[runs a series of before_actions ... and updates the user table]

  ↳ app/controllers/application_controller.rb:166:in `set_site'
  TRANSACTION (0.1ms)  BEGIN
  ↳ app/controllers/application_controller.rb:166:in `set_site'
  User Update (0.4ms)  UPDATE "users" SET "sign_in_count" = $1, "current_sign_in_at" = $2, "last_sign_in_at" = $3, "updated_at" = $4 WHERE "users"."id" = $5  [["sign_in_count", 15], ["current_sign_in_at", "2023-03-14 22:15:30.272009"], ["last_sign_in_at", "2023-03-14 22:13:10.718618"], ["updated_at", "2023-03-14 22:15:30.272227"], ["id", 11]]
  ↳ app/controllers/application_controller.rb:166:in `set_site'
  TRANSACTION (0.5ms)  COMMIT
 ↳ app/controllers/application_controller.rb:166:in `set_site'
  Roleshopuser Load (0.5ms)  SELECT "roleshopusers".* FROM "roleshopusers" WHERE (user_id = 11 AND shop_id = 2) ORDER BY "roleshopusers"."id" ASC LIMIT $1  [["LIMIT", 1]]
  ↳ app/controllers/application_controller.rb:167:in `set_site'
  Shop Pluck (0.2ms)  SELECT "shops"."id" FROM "shops"
  ↳ app/controllers/application_controller.rb:169:in `set_site'
Completed 201 Created in 342ms (Views: 0.1ms | ActiveRecord: 29.0ms | Allocations: 73606)

NoMethodError (undefined method `empty?' for #<User id: 11, email: "redacted", name_last: "X", name_first: "x", admin: true, editor: false,[...]

and renders in the browser the NoMthodError block

There is no single line in the log relating to the sessions controller and the point where this breaks, What can be asserted is: • User.find(11).valid? returns true • the user table is updated • the process breaks lafter this point • the user is not practically logged in (checked by accessing a page restricted to said user. also the application template proposes the sign_in path the !user_signed_in?) - possibly the cookie is not updated?

   <% if !user_signed_in? %>
      <%= link_to fa_icon('sign-in', class: 'fa-2x'), new_user_session_path %>

This conflicts with the session dump flash notice:

[Toggle session dump](http://localhost:3000/users/sign_in#)

flash: {"discard"=>[], "flashes"=>{"notice"=>"Signed in successfully."}}
layout_pref: "photo" # this is correct as set in before_action
locale: :en # this is correct as set in before_action
session_id: "b4184117b87ef05292567b2ad58dfa23"
warden.user.user.key: [[11], "$2a$12$VyeqGfyecobeuVW820Ywb."]

Relevant stack trace

activemodel (7.0.4.2) lib/active_model/attribute_methods.rb:458:in 'method_missing'
rack (2.2.6.4) lib/rack/etag.rb:71:in 'block in digest_body'

nothing in the changelog or upgrade notes refers to this.

Expected behavior

Complete the signin process

carlosantoniodasilva commented 1 year ago

The error looks a lot like https://github.com/heartcombo/devise/issues/5558 and https://github.com/heartcombo/devise/issues/5560, and the problem in those is that they upgraded to Devise 4.9 but didn't remove all previous custom code to workaround Devise + Turbo integration.

I'd recommend taking a look if you have any such custom code, here's a sample upgrade I did on another app someone sent: https://github.com/vincemilo/odin-facebook/pull/1. If you have anything like that, you can probably remove those.

If that is not the case, let me know and we can try some more investigation.

dvodvo commented 1 year ago

I confirm the neutralisation of workarounds allowed to properly deploy devise 4.9.

yoniamir commented 1 year ago

This did the trick for me: https://github.com/heartcombo/devise/issues/5558#issuecomment-1434679268

kulbirsaini commented 3 months ago

This is exactly what I faced upgrading rails from 7.0.2.4 to 7.0.8.3. Here's my diff for devise config for future seekers! I basically regenerated devise config using rails g devise:install and restored my changes that were not Turbo related.

diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index 73a260c..e28b5d6 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -1,22 +1,5 @@
 # frozen_string_literal: true

-# Turbo doesn't work with devise by default.
-# Keep tabs on https://github.com/heartcombo/devise/issues/5446 for a possible fix
-# Fix from https://gorails.com/episodes/devise-hotwire-turbo
-class TurboFailureApp < Devise::FailureApp
-  def respond
-    if request_format == :turbo_stream
-      redirect
-    else
-      super
-    end
-  end
-
-  def skip_format?
-    %w(html turbo_stream */*).include? request_format.to_s
-  end
-end
-
 # Assuming you have not yet modified this file, each configuration option below
 # is set to its default value. Note that some are commented out while others
 # are not: uncommented lines are intended to protect your configuration from
@@ -37,7 +20,6 @@ Devise.setup do |config|
   # ==> Controller configuration
   # Configure the parent class to the devise controllers.
   # config.parent_controller = 'DeviseController'
-  config.parent_controller = 'TurboDeviseController'

   # ==> Mailer Configuration
   # Configure the e-mail address which will be shown in Devise::Mailer,
@@ -282,14 +264,14 @@ Devise.setup do |config|

   # ==> Navigation configuration
   # Lists the formats that should be treated as navigational. Formats like
-  # :html, should redirect to the sign in page when the user does not have
+  # :html should redirect to the sign in page when the user does not have
   # access, but formats like :xml or :json, should return 401.
   #
   # If you have any extra navigational formats, like :iphone or :mobile, you
   # should add them to the navigational formats lists.
   #
   # The "*/*" below is required to match Internet Explorer requests.
-  # config.navigational_formats = ['*/*', :html]
+  # config.navigational_formats = ['*/*', :html, :turbo_stream]
   config.navigational_formats = ['*/*', :html, :turbo_stream]

   # The default HTTP method used to sign out a resource. Default is :delete.
@@ -308,11 +290,6 @@ Devise.setup do |config|
   #   manager.intercept_401 = false
   #   manager.default_strategies(scope: :user).unshift :some_external_strategy
   # end
-  config.warden do |manager|
-    manager.failure_app = TurboFailureApp
-    #   manager.intercept_401 = false
-    #   manager.default_strategies(scope: :user).unshift :some_external_strategy
-  end

   # ==> Mountable engine configurations
   # When using Devise inside an engine, let's call it `MyEngine`, and this engine
@@ -328,12 +305,14 @@ Devise.setup do |config|
   # so you need to do it manually. For the users scope, it would be:
   # config.omniauth_path_prefix = '/my_engine/users/auth'

-  # ==> Turbolinks configuration
-  # If your app is using Turbolinks, Turbolinks::Controller needs to be included to make redirection work correctly:
-  #
-  # ActiveSupport.on_load(:devise_failure_app) do
-  #   include Turbolinks::Controller
-  # end
+  # ==> Hotwire/Turbo configuration
+  # When using Devise with Hotwire/Turbo, the http status for error responses
+  # and some redirects must match the following. The default in Devise for existing
+  # apps is `200 OK` and `302 Found` respectively, but new apps are generated with
+  # these new defaults that match Hotwire/Turbo behavior.
+  # Note: These might become the new default in future versions of Devise.
+  config.responder.error_status = :unprocessable_entity
+  config.responder.redirect_status = :see_other

   # ==> Configuration for :registerable