From f6f367a9510b134428ac1e332fbcdf1917b06188 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Tue, 29 Sep 2020 15:34:33 -0300 Subject: [PATCH] Respect locale set by controller in the failure app A common usage of I18n with different locales is to create some around callback in the application controller that sets the locale for the entire action, via params/url/user/etc., which ensure the locale is respected for the duration of that action, and resets at the end. Devise was not respecting the locale when the authenticate failed and triggered the failure app, because that happens in a warden middleware right up in the change, by that time the controller around callback had already reset the locale back to its default, and the failure app would just translate flash messages using the default locale. Now we are passing the current locale down to the failure app via warden options, and wrapping it with an around callback, which makes the failure app respect the set I18n locale by the controller at the time the authentication failure is triggered, working as expected. (much more like a normal controller would.) I chose to introduce a callback in the failure app so we could wrap the whole `respond` action processing rather than adding individual `locale` options to the `I18n.t` calls, because that should ensure other possible `I18n.t` calls from overridden failure apps would respect the set locale as well, and makes it more like one would implement in a controller. I don't recommend people using callbacks in their own failure apps though, as this is not going to be documented as a "feature" of failures apps, it's considered "internal" and could be refactored at any point. It is possible to override the locale with the new `i18n_locale` method, which simply defaults to the passed locale from the controller. Closes #5247 Closes #5246 Related to: #3052, #4823, and possible others already closed. Related to warden: (may be closed there afterwards) https://github.com/wardencommunity/warden/issues/180 https://github.com/wardencommunity/warden/issues/170 --- CHANGELOG.md | 5 ++++ app/controllers/devise/sessions_controller.rb | 2 +- lib/devise/controllers/helpers.rb | 2 ++ lib/devise/failure_app.rb | 11 +++++++- test/controllers/helpers_test.rb | 10 +++---- test/failure_app_test.rb | 26 +++++++++++++++++++ test/integration/authenticatable_test.rb | 9 +++++++ .../app/controllers/admins_controller.rb | 7 +++++ test/support/locale/pt-BR.yml | 5 ++++ test/test_helper.rb | 2 +- 10 files changed, 71 insertions(+), 8 deletions(-) create mode 100644 test/support/locale/pt-BR.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 26180e5dac..5cab47c219 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +### Unreleased + +* bug fixes + * Respect locale set by controller in failure app. Devise will carry over the current I18n.locale option when triggering authentication, and will wrap the failure app call with it. [#5567](https://github.com/heartcombo/devise/pull/5567) + ### 4.9.3 - 2023-10-11 * enhancements diff --git a/app/controllers/devise/sessions_controller.rb b/app/controllers/devise/sessions_controller.rb index 7c4ee7d4eb..76b780209e 100644 --- a/app/controllers/devise/sessions_controller.rb +++ b/app/controllers/devise/sessions_controller.rb @@ -45,7 +45,7 @@ def serialize_options(resource) end def auth_options - { scope: resource_name, recall: "#{controller_path}#new" } + { scope: resource_name, recall: "#{controller_path}#new", locale: I18n.locale } end def translation_scope diff --git a/lib/devise/controllers/helpers.rb b/lib/devise/controllers/helpers.rb index bc6e9fd865..68e8e8d1d6 100644 --- a/lib/devise/controllers/helpers.rb +++ b/lib/devise/controllers/helpers.rb @@ -46,6 +46,7 @@ def authenticate_#{group_name}!(favorite = nil, opts = {}) mappings.unshift mappings.delete(favorite.to_sym) if favorite mappings.each do |mapping| opts[:scope] = mapping + opts[:locale] = I18n.locale warden.authenticate!(opts) if !devise_controller? || opts.delete(:force) end end @@ -115,6 +116,7 @@ def self.define_helpers(mapping) #:nodoc: class_eval <<-METHODS, __FILE__, __LINE__ + 1 def authenticate_#{mapping}!(opts = {}) opts[:scope] = :#{mapping} + opts[:locale] = I18n.locale warden.authenticate!(opts) if !devise_controller? || opts.delete(:force) end diff --git a/lib/devise/failure_app.rb b/lib/devise/failure_app.rb index 8458aef327..ff3363fbc0 100644 --- a/lib/devise/failure_app.rb +++ b/lib/devise/failure_app.rb @@ -18,6 +18,11 @@ class FailureApp < ActionController::Metal delegate :flash, to: :request + include AbstractController::Callbacks + around_action do |failure_app, action| + I18n.with_locale(failure_app.i18n_locale, &action) + end + def self.call(env) @respond ||= action(:respond) @respond.call(env) @@ -107,7 +112,7 @@ def i18n_message(default = nil) options[:default] = [message] auth_keys = scope_class.authentication_keys keys = (auth_keys.respond_to?(:keys) ? auth_keys.keys : auth_keys).map { |key| scope_class.human_attribute_name(key) } - options[:authentication_keys] = keys.join(I18n.translate(:"support.array.words_connector")) + options[:authentication_keys] = keys.join(I18n.t(:"support.array.words_connector")) options = i18n_options(options) I18n.t(:"#{scope}.#{message}", **options) @@ -116,6 +121,10 @@ def i18n_message(default = nil) end end + def i18n_locale + warden_options[:locale] + end + def redirect_url if warden_message == :timeout flash[:timedout] = true if is_flashing_format? diff --git a/test/controllers/helpers_test.rb b/test/controllers/helpers_test.rb index 655a1fb661..57acdba9c3 100644 --- a/test/controllers/helpers_test.rb +++ b/test/controllers/helpers_test.rb @@ -64,30 +64,30 @@ def setup end test 'proxy authenticate_user! to authenticate with user scope' do - @mock_warden.expects(:authenticate!).with({ scope: :user }) + @mock_warden.expects(:authenticate!).with({ scope: :user, locale: :en }) @controller.authenticate_user! end test 'proxy authenticate_user! options to authenticate with user scope' do - @mock_warden.expects(:authenticate!).with({ scope: :user, recall: "foo" }) + @mock_warden.expects(:authenticate!).with({ scope: :user, recall: "foo", locale: :en }) @controller.authenticate_user!(recall: "foo") end test 'proxy authenticate_admin! to authenticate with admin scope' do - @mock_warden.expects(:authenticate!).with({ scope: :admin }) + @mock_warden.expects(:authenticate!).with({ scope: :admin, locale: :en }) @controller.authenticate_admin! end test 'proxy authenticate_[group]! to authenticate!? with each scope' do [:user, :admin].each do |scope| - @mock_warden.expects(:authenticate!).with({ scope: scope }) + @mock_warden.expects(:authenticate!).with({ scope: scope, locale: :en }) @mock_warden.expects(:authenticate?).with(scope: scope).returns(false) end @controller.authenticate_commenter! end test 'proxy authenticate_publisher_account! to authenticate with namespaced publisher account scope' do - @mock_warden.expects(:authenticate!).with({ scope: :publisher_account }) + @mock_warden.expects(:authenticate!).with({ scope: :publisher_account, locale: :en }) @controller.authenticate_publisher_account! end diff --git a/test/failure_app_test.rb b/test/failure_app_test.rb index 59f291e204..e8f316f0db 100644 --- a/test/failure_app_test.rb +++ b/test/failure_app_test.rb @@ -200,6 +200,13 @@ def call_failure(env_params = {}) assert_equal 'User Steve does not exist', @request.flash[:alert] end + test 'respects the i18n locale passed via warden options when redirecting' do + call_failure('warden' => OpenStruct.new(message: :invalid), 'warden.options' => { locale: :"pt-BR" }) + + assert_equal 'Email ou senha inválidos.', @request.flash[:alert] + assert_equal 'http://test.host/users/sign_in', @response.second["Location"] + end + test 'uses the proxy failure message as string' do call_failure('warden' => OpenStruct.new(message: 'Hello world')) assert_equal 'Hello world', @request.flash[:alert] @@ -284,6 +291,12 @@ def call_failure(env_params = {}) assert_match 'Invalid Email or password.', @response.third.body end + test 'respects the i18n locale passed via warden options when responding to HTTP request' do + call_failure('formats' => Mime[:json], 'warden' => OpenStruct.new(message: :invalid), 'warden.options' => { locale: :"pt-BR" }) + + assert_equal %({"error":"Email ou senha inválidos."}), @response.third.body + end + context 'on ajax call' do context 'when http_authenticatable_on_xhr is false' do test 'dont return 401 with navigational formats' do @@ -372,6 +385,18 @@ def call_failure(env_params = {}) end end + test 'respects the i18n locale passed via warden options when recalling original controller' do + env = { + "warden.options" => { recall: "devise/sessions#new", attempted_path: "/users/sign_in", locale: :"pt-BR" }, + "devise.mapping" => Devise.mappings[:user], + "warden" => stub_everything + } + call_failure(env) + + assert_includes @response.third.body, '

Log in

' + assert_includes @response.third.body, 'Email ou senha inválidos.' + end + # TODO: remove conditional/else when supporting only responders 3.1+ if ActionController::Responder.respond_to?(:error_status=) test 'respects the configured responder `error_status` for the status code' do @@ -431,6 +456,7 @@ def call_failure(env_params = {}) assert_equal "yes it does", Devise::FailureApp.new.lazy_loading_works? end end + context "Without Flash Support" do test "returns to the default redirect location without a flash message" do call_failure request_klass: RequestWithoutFlashSupport diff --git a/test/integration/authenticatable_test.rb b/test/integration/authenticatable_test.rb index 5c56ca589f..ea338f6fc1 100644 --- a/test/integration/authenticatable_test.rb +++ b/test/integration/authenticatable_test.rb @@ -273,6 +273,15 @@ class AuthenticationRedirectTest < Devise::IntegrationTest assert_contain 'You need to sign in or sign up before continuing.' end + test 'redirect from warden respects i18n locale set at the controller' do + get admins_path(locale: "pt-BR") + + assert_redirected_to new_admin_session_path + follow_redirect! + + assert_contain 'Para continuar, faça login ou registre-se.' + end + test 'redirect to default url if no other was configured' do sign_in_as_user assert_template 'home/index' diff --git a/test/rails_app/app/controllers/admins_controller.rb b/test/rails_app/app/controllers/admins_controller.rb index c732f58908..957aa6f0b2 100644 --- a/test/rails_app/app/controllers/admins_controller.rb +++ b/test/rails_app/app/controllers/admins_controller.rb @@ -1,8 +1,15 @@ # frozen_string_literal: true class AdminsController < ApplicationController + around_action :set_locale before_action :authenticate_admin! def index end + + private + + def set_locale + I18n.with_locale(params[:locale] || I18n.default_locale) { yield } + end end diff --git a/test/support/locale/pt-BR.yml b/test/support/locale/pt-BR.yml new file mode 100644 index 0000000000..5c57e19096 --- /dev/null +++ b/test/support/locale/pt-BR.yml @@ -0,0 +1,5 @@ +pt-BR: + devise: + failure: + invalid: "%{authentication_keys} ou senha inválidos." + unauthenticated: "Para continuar, faça login ou registre-se." diff --git a/test/test_helper.rb b/test/test_helper.rb index c0bb43f779..199dad4043 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -10,7 +10,7 @@ require "rails/test_help" require "orm/#{DEVISE_ORM}" -I18n.load_path << File.expand_path("../support/locale/en.yml", __FILE__) +I18n.load_path.concat Dir["#{File.dirname(__FILE__)}/support/locale/*.yml"] require 'mocha/minitest' require 'timecop'