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

Warden doesn't preserve locale when thrown #180

Open
JoeWoodward opened this issue Oct 29, 2019 · 9 comments
Open

Warden doesn't preserve locale when thrown #180

JoeWoodward opened this issue Oct 29, 2019 · 9 comments
Assignees
Labels

Comments

@JoeWoodward
Copy link

We have been experiencing an issue where the wrong locale is being used for error messages returned from the Devise FailureApp, I believe it's due to the way that warden throws and catches.

The recommended way of setting the locale for a rails controller action is to use an around_action callback, e.g.

prepend_around_action :with_locale

def with_locale(action)
  I18n.with_locale(current_locale) { action }
end

When warden fails to authenticate it throws and calls a rack endpoint, the failure app... This means that the around_action is exited so the locale is no longer applied.

I don't see any way to tell warden to preserve the locale that was applied when the throw was called. Should warden do this internally?

@JoeWoodward
Copy link
Author

Or maybe this should be handled by Devise? Perhaps devise should be passing the current locale into the throw call

@JoeWoodward
Copy link
Author

Related to heartcombo/devise#4823

@JoeWoodward
Copy link
Author

Actually it's not devise that throws, it's warden in the authenticate! method

@jsmestad jsmestad self-assigned this Oct 29, 2019
@jsmestad
Copy link
Collaborator

I read through the related devise thread. It is important to remember that Warden only deals with Rack. We intentionally avoid any reliance on Rails (ala around_action) because Warden works with all Rack-based frameworks (Rails, Sinatra, Hanami, Grape, etc).

Did you try the changes that were suggested in platformatec/devise#4823 ? It appears from that discussion that changing the middleware ordering will resolve the issue.

@JoeWoodward
Copy link
Author

This is not related to the middleware stack in my case. I believe other users in that thread also have the same issue and failed to locate the problem. We have resolved the issue by using a custom failure app and manually setting the locale based on the params but this solution is suboptimal as the locale might not always come from the request params.

Another issue we're facing now is that this custom failure app isn't being used while Warden.test_mode! is activated. Looking into a fix for that now.

From reading through the docs I have a feeling there is a solution to the problem that won't require any changes to warden or devise. Will investigate further and report back

@JasonBarnabe
Copy link

I18n::Middleware discussed in heartcombo/devise#4823 serves only to reset the locale after the request completes. It's not about setting the correct locale.

The problem here is specific to the use of I18n.with_locale. I've run a number of tests with the "Invalid email/password" message.

Set I18n.locale in before_action in application_controller.rb:

    before_action do
      I18n.locale = :es
    end

Works.

Set I18n.locale with middleware:

class LocaleMiddleware
  def initialize(app)
    @app = app
  end

  def call(env)
    I18n.locale = :es
    status, headers, body = @app.call(env)
  end
end
config.middleware.use ::LocaleMiddleware

Works.

Use I18n.locale in around_action in application_controller.rb:

around_action :test_set_locale
  def test_set_locale(&action)
    I18n.with_locale(:es, &action)
  end

Doesn't work for the error message, works for the rest of the app.

Use I18n.locale in middleware:

class LocaleMiddleware
  def initialize(app)
    @app = app
  end

  def call(env)
    I18n.with_locale(:es) do
      status, headers, body = @app.call(env)
    end
  end
end
config.middleware.use ::LocaleMiddleware

Doesn't work for anything.

@nov
Copy link

nov commented Sep 28, 2022

(double posting of heartcombo/devise#5247 (comment))

I also had the same issue, and resulted in new rack middleware which handles query param, cookie and HTTP_ACCEPT_LANGUAGE header in this order, then store explicitly requested locale in cookie.
https://github.com/nov/rack-locale_memorable

ps.
Rack::Locale wasn't enough for our use-case since it does handle only HTTP_ACCEPT_LANGUAGE header.

carlosantoniodasilva added a commit to heartcombo/devise 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 to heartcombo/devise 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
Copy link
Contributor

Hey, I finally have a PR up with this change on the Devise side of things: heartcombo/devise#5567, basically carrying over the locale from the request to warden and wrapping the failure app with it. If you happen to still be having this issue and want to give it a shot, let me know if you run into any issues.

carlosantoniodasilva added a commit to heartcombo/devise 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 to heartcombo/devise 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 to heartcombo/devise 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 to heartcombo/devise 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 to heartcombo/devise 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 to heartcombo/devise 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 to heartcombo/devise 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 to heartcombo/devise 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 to heartcombo/devise 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
@RavWar
Copy link

RavWar commented Mar 19, 2024

Just stumbled upon this issue and the latest PR did not help in my case at first, but after also adding this fix: heartcombo/devise#5602 (comment) - everything works as expected

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

No branches or pull requests

6 participants