From a71c5de63fa1d1b62318a33e7dec0222e2b84c43 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Tue, 31 Jan 2023 09:32:40 -0300 Subject: [PATCH] Integrate with Hotwire/Turbo by configuring error and response statuses Treat `:turbo_stream` request format as a navigational format, much like HTML, so Devise/responders can work properly. Allow configuring the `error_status` and `redirect_status` using the latest responders features, via a new custom Devise responder, so we can customize the both responses to match Hotwire/Turbo behavior, for example with `422 Unprocessable Entity` and `303 See Other`, respectively. The defaults aren't changing in Devise itself (yet), so it still responds on errors cases with `200 OK`, and redirects on non-GET requests with `302 Found`, but new apps are generated with the new statuses and existing apps can opt-in. Please note that these defaults might change in a future release of Devise. PRs/Issues references: https://github.com/heartcombo/devise/pull/5545 https://github.com/heartcombo/devise/pull/5529 https://github.com/heartcombo/devise/pull/5516 https://github.com/heartcombo/devise/pull/5499 https://github.com/heartcombo/devise/pull/5487 https://github.com/heartcombo/devise/pull/5467 https://github.com/heartcombo/devise/pull/5440 https://github.com/heartcombo/devise/pull/5410 https://github.com/heartcombo/devise/pull/5340 https://github.com/heartcombo/devise/issues/5542 https://github.com/heartcombo/devise/issues/5530 https://github.com/heartcombo/devise/issues/5519 https://github.com/heartcombo/devise/issues/5513 https://github.com/heartcombo/devise/issues/5478 https://github.com/heartcombo/devise/issues/5468 https://github.com/heartcombo/devise/issues/5463 https://github.com/heartcombo/devise/issues/5458 https://github.com/heartcombo/devise/issues/5448 https://github.com/heartcombo/devise/issues/5446 https://github.com/heartcombo/devise/issues/5439 --- CHANGELOG.md | 1 + Gemfile | 2 +- Gemfile.lock | 14 ++++++++--- README.md | 19 ++++++++++++++ .../devise/confirmations_controller.rb | 1 + .../devise/registrations_controller.rb | 2 +- app/controllers/devise/sessions_controller.rb | 2 +- app/controllers/devise/unlocks_controller.rb | 1 + app/controllers/devise_controller.rb | 1 + app/views/devise/registrations/edit.html.erb | 2 +- gemfiles/Gemfile-rails-6-0 | 2 +- gemfiles/Gemfile-rails-6-1 | 2 +- gemfiles/Gemfile-rails-main | 2 +- lib/devise.rb | 10 +++++++- lib/devise/controllers/responder.rb | 25 +++++++++++++++++++ lib/devise/failure_app.rb | 6 +++-- lib/generators/templates/devise.rb | 18 +++++++------ .../registrations/edit.html.erb | 2 +- test/support/integration.rb | 4 +-- 19 files changed, 91 insertions(+), 25 deletions(-) create mode 100644 lib/devise/controllers/responder.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d48a34551..4636ddfa06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * enhancements * Add support for Ruby 3.1/3.2. + * Add support for Hotwire + Turbo, default in Rails 7+. ### 4.8.1 - 2021-12-16 diff --git a/Gemfile b/Gemfile index 0b41fa0e1b..aac13712ca 100644 --- a/Gemfile +++ b/Gemfile @@ -11,7 +11,7 @@ gem "rdoc" gem "rails-controller-testing", github: "rails/rails-controller-testing" -gem "responders", "~> 3.0" +gem "responders", github: "heartcombo/responders", branch: "main" group :test do gem "nokogiri", "< 1.13" diff --git a/Gemfile.lock b/Gemfile.lock index 704f50e976..a9a79c79a7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,12 @@ +GIT + remote: https://github.com/heartcombo/responders.git + revision: fb9f787055a7a842584ce351793b249676290090 + branch: main + specs: + responders (3.0.1) + actionpack (>= 5.2) + railties (>= 5.2) + GIT remote: https://github.com/rails/rails-controller-testing.git revision: 351c0162df0771c0c48e6a5a886c4c2f0a5d1a74 @@ -189,9 +198,6 @@ GEM rake (13.0.6) rdoc (6.5.0) psych (>= 4.0.0) - responders (3.0.1) - actionpack (>= 5.0) - railties (>= 5.0) rexml (3.2.5) ruby-openid (2.9.2) ruby2_keywords (0.0.5) @@ -231,7 +237,7 @@ DEPENDENCIES rails (~> 7.0.0) rails-controller-testing! rdoc - responders (~> 3.0) + responders! rexml sqlite3 (~> 1.4) timecop diff --git a/README.md b/README.md index 8fa3f64b9d..cd2e74a830 100644 --- a/README.md +++ b/README.md @@ -476,6 +476,25 @@ Please note: You will still need to add `devise_for` in your routes in order to devise_for :users, skip: :all ``` +### Hotwire/Turbo + +Devise integrates with Hotwire/Turbo by treating such requests as navigational, and configuring certain responses for errors and redirects to match the expected behavior. New apps are generated with the following response configuration by default, and existing apps may opt-in by adding the config to their Devise initializers: + +```ruby +Devise.setup do |config| + # ... + # 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 +end +``` + +_Note_: the above statuses configuration may become the default for Devise in a future release. + ### I18n Devise uses flash messages with I18n, in conjunction with the flash keys :notice and :alert. To customize your app, you can set up your locale file: diff --git a/app/controllers/devise/confirmations_controller.rb b/app/controllers/devise/confirmations_controller.rb index 3069c6efbf..5e22079ec8 100644 --- a/app/controllers/devise/confirmations_controller.rb +++ b/app/controllers/devise/confirmations_controller.rb @@ -27,6 +27,7 @@ def show set_flash_message!(:notice, :confirmed) respond_with_navigational(resource){ redirect_to after_confirmation_path_for(resource_name, resource) } else + # TODO: use `error_status` when the default changes to `:unprocessable_entity`. respond_with_navigational(resource.errors, status: :unprocessable_entity){ render :new } end end diff --git a/app/controllers/devise/registrations_controller.rb b/app/controllers/devise/registrations_controller.rb index 1b8a969f82..f1292b4d90 100644 --- a/app/controllers/devise/registrations_controller.rb +++ b/app/controllers/devise/registrations_controller.rb @@ -67,7 +67,7 @@ def destroy Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name) set_flash_message! :notice, :destroyed yield resource if block_given? - respond_with_navigational(resource){ redirect_to after_sign_out_path_for(resource_name) } + respond_with_navigational(resource){ redirect_to after_sign_out_path_for(resource_name), status: Devise.responder.redirect_status } end # GET /resource/cancel diff --git a/app/controllers/devise/sessions_controller.rb b/app/controllers/devise/sessions_controller.rb index 9090b002f3..7c4ee7d4eb 100644 --- a/app/controllers/devise/sessions_controller.rb +++ b/app/controllers/devise/sessions_controller.rb @@ -77,7 +77,7 @@ def respond_to_on_destroy # support returning empty response on GET request respond_to do |format| format.all { head :no_content } - format.any(*navigational_formats) { redirect_to after_sign_out_path_for(resource_name) } + format.any(*navigational_formats) { redirect_to after_sign_out_path_for(resource_name), status: Devise.responder.redirect_status } end end end diff --git a/app/controllers/devise/unlocks_controller.rb b/app/controllers/devise/unlocks_controller.rb index 1d04d62274..b1487760b6 100644 --- a/app/controllers/devise/unlocks_controller.rb +++ b/app/controllers/devise/unlocks_controller.rb @@ -29,6 +29,7 @@ def show set_flash_message! :notice, :unlocked respond_with_navigational(resource){ redirect_to after_unlock_path_for(resource) } else + # TODO: use `error_status` when the default changes to `:unprocessable_entity`. respond_with_navigational(resource.errors, status: :unprocessable_entity){ render :new } end end diff --git a/app/controllers/devise_controller.rb b/app/controllers/devise_controller.rb index 9911fa0b85..4bf92a6887 100644 --- a/app/controllers/devise_controller.rb +++ b/app/controllers/devise_controller.rb @@ -15,6 +15,7 @@ class DeviseController < Devise.parent_controller.constantize end prepend_before_action :assert_is_devise_resource! + self.responder = Devise.responder respond_to :html if mimes_for_respond_to.empty? # Override prefixes to consider the scoped view. diff --git a/app/views/devise/registrations/edit.html.erb b/app/views/devise/registrations/edit.html.erb index 38d95b85a8..2a666e9257 100644 --- a/app/views/devise/registrations/edit.html.erb +++ b/app/views/devise/registrations/edit.html.erb @@ -38,6 +38,6 @@

Cancel my account

-

Unhappy? <%= button_to "Cancel my account", registration_path(resource_name), data: { confirm: "Are you sure?" }, method: :delete %>

+

Unhappy? <%= button_to "Cancel my account", registration_path(resource_name), data: { confirm: "Are you sure?", turbo_confirm: "Are you sure?" }, method: :delete %>

<%= link_to "Back", :back %> diff --git a/gemfiles/Gemfile-rails-6-0 b/gemfiles/Gemfile-rails-6-0 index d8f7743151..b6cb38f8ce 100644 --- a/gemfiles/Gemfile-rails-6-0 +++ b/gemfiles/Gemfile-rails-6-0 @@ -9,7 +9,7 @@ gem "rdoc" gem "rails-controller-testing", github: "rails/rails-controller-testing" -gem "responders", "~> 3.0" +gem "responders", github: "heartcombo/responders", branch: "main" group :test do gem "nokogiri", "< 1.13" diff --git a/gemfiles/Gemfile-rails-6-1 b/gemfiles/Gemfile-rails-6-1 index 49b19f342d..1dc626ed7f 100644 --- a/gemfiles/Gemfile-rails-6-1 +++ b/gemfiles/Gemfile-rails-6-1 @@ -9,7 +9,7 @@ gem "rdoc" gem "rails-controller-testing", github: "rails/rails-controller-testing" -gem "responders", "~> 3.0" +gem "responders", github: "heartcombo/responders", branch: "main" if RUBY_VERSION >= "3.1" gem "net-smtp", require: false diff --git a/gemfiles/Gemfile-rails-main b/gemfiles/Gemfile-rails-main index 4104445e75..fe9eba5aff 100644 --- a/gemfiles/Gemfile-rails-main +++ b/gemfiles/Gemfile-rails-main @@ -9,7 +9,7 @@ gem "rdoc" gem "rails-controller-testing", github: "rails/rails-controller-testing" -gem "responders", "~> 3.0" +gem "responders", github: "heartcombo/responders", branch: "main" group :test do gem "nokogiri", "< 1.13" diff --git a/lib/devise.rb b/lib/devise.rb index ede8038679..55045a8c18 100644 --- a/lib/devise.rb +++ b/lib/devise.rb @@ -23,6 +23,7 @@ module Devise module Controllers autoload :Helpers, 'devise/controllers/helpers' autoload :Rememberable, 'devise/controllers/rememberable' + autoload :Responder, 'devise/controllers/responder' autoload :ScopedViews, 'devise/controllers/scoped_views' autoload :SignInOut, 'devise/controllers/sign_in_out' autoload :StoreLocation, 'devise/controllers/store_location' @@ -217,7 +218,14 @@ module Test # Which formats should be treated as navigational. mattr_accessor :navigational_formats - @@navigational_formats = ["*/*", :html] + @@navigational_formats = ["*/*", :html, :turbo_stream] + + # The default responder used by Devise, not meant to be changed directly, + # but you can customize status codes with: + # `config.responder.error_status` + # `config.responder.redirect_status` + mattr_accessor :responder + @@responder = Devise::Controllers::Responder # When set to true, signing out a user signs out all other scopes. mattr_accessor :sign_out_all_scopes diff --git a/lib/devise/controllers/responder.rb b/lib/devise/controllers/responder.rb new file mode 100644 index 0000000000..c0579fc68f --- /dev/null +++ b/lib/devise/controllers/responder.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Devise + module Controllers + # Custom Responder to configure default statuses that only apply to Devise, + # and allow to integrate more easily with Hotwire/Turbo. + class Responder < ActionController::Responder + if respond_to?(:error_status=) && respond_to?(:redirect_status=) + self.error_status = :ok + self.redirect_status = :found + else + # TODO: remove this support for older Rails versions, which aren't supported by Turbo + # and/or responders. It won't allow configuring a custom response, but it allows Devise + # to use these methods and defaults across the implementation more easily. + def self.error_status + :ok + end + + def self.redirect_status + :found + end + end + end + end +end \ No newline at end of file diff --git a/lib/devise/failure_app.rb b/lib/devise/failure_app.rb index ee8219fff1..d8042ec318 100644 --- a/lib/devise/failure_app.rb +++ b/lib/devise/failure_app.rb @@ -71,7 +71,9 @@ def recall end flash.now[:alert] = i18n_message(:invalid) if is_flashing_format? - self.response = recall_app(warden_options[:recall]).call(request.env) + self.response = recall_app(warden_options[:recall]).call(request.env).tap { |response| + response[0] = Rack::Utils.status_code(Devise.responder.error_status) + } end def redirect @@ -167,7 +169,7 @@ def scope_url end def skip_format? - %w(html */*).include? request_format.to_s + %w(html */* turbo_stream).include? request_format.to_s end # Choose whether we should respond in an HTTP authentication fashion, diff --git a/lib/generators/templates/devise.rb b/lib/generators/templates/devise.rb index 1dbaddaa6e..4503f33f81 100644 --- a/lib/generators/templates/devise.rb +++ b/lib/generators/templates/devise.rb @@ -256,14 +256,14 @@ # ==> 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] # The default HTTP method used to sign out a resource. Default is :delete. config.sign_out_via = :delete @@ -296,12 +296,14 @@ # 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 diff --git a/lib/generators/templates/simple_form_for/registrations/edit.html.erb b/lib/generators/templates/simple_form_for/registrations/edit.html.erb index dfb7eb94ea..147c6a0efb 100644 --- a/lib/generators/templates/simple_form_for/registrations/edit.html.erb +++ b/lib/generators/templates/simple_form_for/registrations/edit.html.erb @@ -30,6 +30,6 @@

Cancel my account

-

Unhappy? <%= link_to "Cancel my account", registration_path(resource_name), data: { confirm: "Are you sure?" }, method: :delete %>

+

Unhappy? <%= button_to "Cancel my account", registration_path(resource_name), data: { confirm: "Are you sure?", turbo_confirm: "Are you sure?" }, method: :delete %>

<%= link_to "Back", :back %> diff --git a/test/support/integration.rb b/test/support/integration.rb index 76d297a499..3ed0d85aaf 100644 --- a/test/support/integration.rb +++ b/test/support/integration.rb @@ -61,8 +61,8 @@ def sign_in_as_admin(options = {}, &block) # account Middleware redirects. # def assert_redirected_to(url) - assert_includes [301, 302], @integration_session.status, - "Expected status to be 301 or 302, got #{@integration_session.status}" + assert_includes [301, 302, 303], @integration_session.status, + "Expected status to be 301, 302, or 303, got #{@integration_session.status}" assert_url url, @integration_session.headers["Location"] end