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

Wrong locale (I18n) in failure_app #4823

Closed
Yegorov opened this issue Mar 30, 2018 · 10 comments
Closed

Wrong locale (I18n) in failure_app #4823

Yegorov opened this issue Mar 30, 2018 · 10 comments

Comments

@Yegorov
Copy link

Yegorov commented Mar 30, 2018

Environment

  • Ruby 2.4.1
  • Rails 5.1.6
  • Devise 4.4.3

Current behavior

I set locale in my ApplicationController, as this:

class ApplicationController < ActionController::Base
  before_action :set_locale
  # other code
  def set_locale
    I18n.locale = :ru
  end
end

And if i submit wrong email with password to Users::SessionsController#create, flash mesage has :en locale.

For example, i monkey patched add binding.pry to this, and i get in console:

    376:     def i18n_message(default = nil)
    377:       message = warden_message || default || :unauthenticated
    378: 
    379:       if message.is_a?(Symbol)
    380:         options = {}
    381:         options[:resource_name] = scope
    382:         options[:scope] = "devise.failure"
    383:         options[:default] = [message]
    384:         auth_keys = scope_class.authentication_keys
    385:         keys = (auth_keys.respond_to?(:keys) ? auth_keys.keys : auth_keys).map { |key| scope_class.human_attribute_name(key) }
    386:         options[:authentication_keys] = keys.join(I18n.translate(:"support.array.words_connector"))
    387:         options = i18n_options(options)
 => 388: binding.pry
    389:         I18n.t(:"#{scope}.#{message}", options)
    390:       else
    391:         message.to_s
    392:       end
    393:     end

[1] pry(#<Devise::FailureApp>)> I18n.locale
=> :en

What could be wrong?

Other translate flash message in devise work ok!

Expected behavior

I18n.locale must be one that is set to ApplicationController#set_locale

@tegon
Copy link
Member

tegon commented Apr 6, 2018

Hello @Yegorov, thanks for your report.

That's odd, I've just tested here with a new application - with the same devise and rails versions - and the flash message was displayed correctly.
screen shot 2018-04-06 at 18 22 12

Are you overriding one of devise's controller, or do you have any other code that might be changing the locale?

Here's the application I used if you want to try it too:
i18n-failure-app.zip

@Yegorov
Copy link
Author

Yegorov commented Apr 7, 2018

Hi @tegon. Thanks for your response!
If this matters, then I use Rack::Locale middleware to determine the current user locale.
I modified your test application (i18n-failure-app_2.zip).
To reproduce the error you need repeatedly change the locale on the main page, go to the login page and enter the wrong data.
Some screenshots for proof:
_478
_479

@rajraj
Copy link

rajraj commented Apr 16, 2018

I can confirm this is happening with a rails 5.1.5 / devise 4.4.3 app.

We are overriding the devise's registrations and sessions controllers in our app.

@tegon
Copy link
Member

tegon commented May 10, 2018

@Yegorov Since you're using a middleware to change the locale, I think the problem may be because it's running after Warden's middleware - which calls the Failure application.
Inside the example application, when I run rails middleware, I get the following output:

use Rack::Sendfile
use ActionDispatch::Static
use ActionDispatch::Executor
use ActiveSupport::Cache::Strategy::LocalCache::Middleware
use Rack::Runtime
use Rack::MethodOverride
use ActionDispatch::RequestId
use ActionDispatch::RemoteIp
use Sprockets::Rails::QuietAssets
use Rails::Rack::Logger
use ActionDispatch::ShowExceptions
use WebConsole::Middleware
use ActionDispatch::DebugExceptions
use ActionDispatch::Reloader
use ActionDispatch::Callbacks
use ActiveRecord::Migration::CheckPending
use ActionDispatch::Cookies
use ActionDispatch::Session::CookieStore
use ActionDispatch::Flash
use Rack::Head
use Rack::ConditionalGet
use Rack::ETag
use Warden::Manager
use Rack::Locale
run I18nFailureApp::Application.routes

Notice that the locale middleware runs after Warden. Now, if I change config/application.rb to:

    config.middleware.insert_before Warden::Manager, Rack::Locale

This is rails middleware output:

use ActionDispatch::Static
use ActionDispatch::Executor
use ActiveSupport::Cache::Strategy::LocalCache::Middleware
use Rack::Runtime
use Rack::MethodOverride
use ActionDispatch::RequestId
use ActionDispatch::RemoteIp
use Sprockets::Rails::QuietAssets
use Rails::Rack::Logger
use ActionDispatch::ShowExceptions
use WebConsole::Middleware
use ActionDispatch::DebugExceptions
use ActionDispatch::Reloader
use ActionDispatch::Callbacks
use ActiveRecord::Migration::CheckPending
use ActionDispatch::Cookies
use ActionDispatch::Session::CookieStore
use ActionDispatch::Flash
use Rack::Head
use Rack::ConditionalGet
use Rack::ETag
use Rack::Locale
use Warden::Manager
run I18nFailureApp::Application.routes

I've tested with this change and everything worked. Can you test too?

@zinkkrysty
Copy link

zinkkrysty commented May 14, 2018

I have the same issue in rails 4.2: the locale is reset to :en in the FailureApp

I, however, do not have Rack::Locale in my middleware stack so I cannot use the workaround that @tegon suggested

EDIT I noticed you are using Rack::Locale specifically to set locale. I am setting locale in a before_filter, and that does not work with the FailureApp. Any workaround which I could try in my case?

EDIT no. 2: I managed to replace the FailureApp with a custom one:

  config.warden do |manager|
    manager.failure_app = CustomerUserFailure
  end
class CustomerUserFailure < Devise::FailureApp
  def respond
    # Set the locale from params (minimal requirement to get Devise working
    # with our current locale setup)
    I18n.locale = params[:locale]

    super
  end
end

This works for me. In my opinion, there should be a guide for setting this up, the localization does not work as expected straight from the box with Devise.

@tegon
Copy link
Member

tegon commented May 14, 2018

@zinkkrysty If that worked for you, feel free to add a page to the Wiki with your solution. That's the Wiki purpose: it's maintained by the community and aggregates solutions to some problems.

Seems like every app will have different ways to change the current locale, but I don't know if we can do something different here. We simply call I18n.t from the failure application, so as long as the I18n.locale is properly set at the time we call the failure application, everything should be fine.

@fabn
Copy link

fabn commented May 14, 2018

Same thing happening here. I'm managing locale in this way and all messages but warden ones are translated correctly.

DeviseController.prepend_around_action :switch_locale
  # Change locale for this action
  def switch_locale(&action)
    I18n.with_locale(page_locale, &action)
  end

@tegon
Copy link
Member

tegon commented Aug 1, 2018

So, I was looking into this again today and it seems like it's an i18n known issue for multi-threaded environments. See the related issue: ruby-i18n/i18n#381.
If you add the middleware introduced in ruby-i18n/i18n#382 this should probably work as expected.

config.middleware.insert_before Warden::Manager, I18n::Middleware

Also, another thing that was mentioned there is that calling I18n.locale = directly is not thread-safe, is better to use I18n.with_locale from a controller around_action.

I'm going to close this since it's not a Devise issue because we're only calling I18n.t normally.

@tegon tegon closed this as completed Aug 1, 2018
@molenick
Copy link

molenick commented Aug 7, 2018

I'm having a similar issue and didn't have success using config.middleware.insert_before Warden::Manager, I18n::Middleware.

  1. User 1 accesses a non-auth resource with a locale (en - English).
  2. User 2 accesses any resource with a different locale (es - Spanish).
  3. User 1 accesses a resource that requires authentication and is redirected to the locale of User 2's last request ignoring User 1's locale - the whole page is translated to Spanish instead of English.

With config.middleware.use I18n::Middleware set, User 1 is redirected to the correct stored locale after a successful sign-in.

With config.middleware.insert_before Warden::Manager, I18n::Middleware, they are redirected to the incorrect locale of User 2's request upon successful sign-in.

It seems like this could be fixed by adding
opts[:locale] = params[:locale]
inside scope_url so that devise always redirects to the original locale through the sign-in process.

Thanks!

@tegon
Copy link
Member

tegon commented Aug 7, 2018

@molenick

With config.middleware.use I18n::Middleware set, User 1 is redirected to the correct stored locale after a successful sign-in.

Seems like I've typed wrongly in there with the insert_before. It makes sense, because the I18n middleware cleans the locale, so what you did with config.middleware.use I18n::Middleware is the right way.

It seems like this could be fixed by adding
opts[:locale] = params[:locale]
inside scope_url so that devise always redirects to the original locale through the sign-in process.

We can't do that because not every application uses params[:locale] to translate. Someone can use a different parameter e.g. lang or even use another logic to translate a page - base on a database column, for example.

It's important to say here that this is not a Devise issue. We call I18n.t as any other application would. It's a matter of correctly mixing the middlewares order and the code that switches the locale in the application.

carlosantoniodasilva added a commit that referenced this issue Mar 3, 2023
A common usage of I18n with different locales is to create some around
callbcak 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)
wardencommunity/warden#180
wardencommunity/warden#170
carlosantoniodasilva added a commit that referenced this issue Mar 7, 2023
A common usage of I18n with different locales is to create some around
callbcak 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)
wardencommunity/warden#180
wardencommunity/warden#170
carlosantoniodasilva added a commit that referenced this issue Mar 17, 2023
A common usage of I18n with different locales is to create some around
callbcak 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)
wardencommunity/warden#180
wardencommunity/warden#170
carlosantoniodasilva added a commit that referenced this issue Mar 30, 2023
A common usage of I18n with different locales is to create some around
callbcak 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)
wardencommunity/warden#180
wardencommunity/warden#170
carlosantoniodasilva added a commit that referenced this issue Jun 27, 2023
A common usage of I18n with different locales is to create some around
callbcak 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)
wardencommunity/warden#180
wardencommunity/warden#170
carlosantoniodasilva added a commit that referenced this issue Oct 12, 2023
A common usage of I18n with different locales is to create some around
callbcak 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)
wardencommunity/warden#180
wardencommunity/warden#170
carlosantoniodasilva added a commit that referenced this issue Oct 12, 2023
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)
wardencommunity/warden#180
wardencommunity/warden#170
carlosantoniodasilva added a commit that referenced this issue Oct 12, 2023
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)
wardencommunity/warden#180
wardencommunity/warden#170
carlosantoniodasilva added a commit that referenced this issue Oct 13, 2023
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)
wardencommunity/warden#180
wardencommunity/warden#170
carlosantoniodasilva added a commit that referenced this issue Oct 13, 2023
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)
wardencommunity/warden#180
wardencommunity/warden#170
carlosantoniodasilva added a commit that referenced this issue Oct 13, 2023
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)
wardencommunity/warden#180
wardencommunity/warden#170
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants