Respect locale set by controller in the failure app

[carlosantoniodasilva]

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

[lapser]

Please commit/merge this asap if it works.

[carlosantoniodasilva]

@lapser thanks. I plan to get it merged soon, just leaving it sit here for a bit longer in case someone from the issues I pinged previously wanted to give it a try first. If you do test it out, let me know how it goes.

[ejohnson6]

I was having the same issue as https://github.com/heartcombo/devise/issues/5246 and tried out this branch and confirmed that it fixes the issue for me

[lillieatwork]

This would be wildly helpful!! ❤️ 🚢 :shipit: ❤️

[hientranea]

Any updates on this PR? I had the same problem and the PR fixes the issue for me 🚀

[mark-young-atg]

This looks good and works for me too. If it isn't ready to be merged right away, can it be rebased against the latest release (4.9.2) so anyone using this branch is getting the latest devise plus this improvement?

[carlosantoniodasilva]

@mark-young-atg this is now rebased against v4.9.2 (not main -- I have to review a couple of things that got in there recently that I haven't had the chance to yet)

Everyone else who commented: I'll get this merged and hopefully released soon, appreciate the help confirming it works as expected for you all, thanks for the patience.

[mark-young-atg]

I have removed my previous comments. This branch does work as expected. The problem I saw is elsewhere.

[leonardow-unep-wcmc]

Having the same issue and this PR works for me. Rails 7.0.5.1 API only app Devise 4.2.9

[ejohnson6]

Hey just checking on this again, hopping to see it get in soon! Any progress updates would be appreciated! 😃

[carlosantoniodasilva]

Sorry folks, I know it's been taking some time, but I had to step away from things for personal reasons in the past couple of months. I'm slowly catching up now, and have this as one of the main things on my radar to look at. :soon:

[mark-young-atg]

Hi @carlosantoniodasilva I trust you are well. I see you are getting active again with devise. Is there any chance of getting this PR merged in as part of v4.9.3? All the best, Mark

[carlosantoniodasilva]

@mark-young-atg :wave: thanks.

v4.9.3 already landed, but I'll get this merged and released as part of the next version.

[carlosantoniodasilva]

This is in 4-stable and main now.

[lillieatwork]

Thank you for all your work on this project!! 💖 💖 💖

[mark-young-atg]

Thank you @carlosantoniodasilva, much appreciated. What is the timescale for the next release?

[yuhiyone]

@carlosantoniodasilva Thank you for your work. I am also facing the same problem with my app. I'm looking forward to a new issue.

[mark-young-atg]

Now this work has been merged into main is it possible to produce a new release of devise please?

[RavWar]

Maybe it's worth noting, if current_user is called anywhere in before action then this fix is not working, sign_in goes straight to warden and sessions_controller#create action along with auth_options are not called. Combining with this fix: https://github.com/heartcombo/devise/issues/5602#issuecomment-1876164084 - everything works as expected

[carlosantoniodasilva]

@RavWar the ideal solution is to make sure you're always setting the locale first, in a prepended before action / around action (this is true not only for Devise, but as a general rule, if your app depends on I18n locale to be set). That other change you pointed out can be used, but it has more side-effects that are worth being aware of.

@mark-young-atg I'll look into cutting a new release, however from 4-stable (since main contains other unrelated / breaking changes), thanks.

[carlosantoniodasilva]

For folks waiting on a release: I just released v4.9.4 including this fix.