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

Fix locale for authfail to be consistant with g:message FormatTagLib resolveLocale #1025

Merged
merged 1 commit into from
Oct 11, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package grails.plugin.springsecurity

import grails.converters.JSON
import org.grails.web.servlet.mvc.GrailsWebRequest
import org.springframework.context.MessageSource
import org.springframework.security.access.annotation.Secured
import org.springframework.security.authentication.AccountExpiredException
Expand Down Expand Up @@ -104,18 +105,19 @@ class LoginController {
String msg = ''
def exception = session[WebAttributes.AUTHENTICATION_EXCEPTION]
if (exception) {
def locale = GrailsWebRequest.lookup().getLocale() ?: Locale.getDefault()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of GrailsWebRequest.lookup().getLocale() ?: Locale.getDefault() I think you could just use LocaleContextHolder.locale which should give you the same result.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matrei "should" ? I took the logic from FormatTagLib.groovy#L336-L349
can you confirm 100% it will give the same result. If so, FormatTagLib would need to be updated as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Should" as in I have not tested it in this context.

According to https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/context/i18n/LocaleContextHolder.html#getLocale():

Return the Locale associated with the current thread, if any, or the system default Locale otherwise

From https://docs.spring.io/spring-framework/reference/web/webmvc/mvc-servlet/localeresolver.html:

When a request comes in, the DispatcherServlet looks for a locale resolver and, if it finds one, it tries to use it to set the locale.

By default in a Grails app, that is a SessionLocaleResolver, set by GrailsI18nPlugin.doWithSpring().

GrailsWebRequest.lookup().getLocale() will eventually end up using the same LocaleResolver to get the Locale but it will not fallback to using the system Locale.

if (exception instanceof AccountExpiredException) {
msg = messageSource.getMessage('springSecurity.errors.login.expired', null, "Account Expired", request.locale)
msg = messageSource.getMessage('springSecurity.errors.login.expired', null, "Account Expired", locale)
} else if (exception instanceof CredentialsExpiredException) {
msg = messageSource.getMessage('springSecurity.errors.login.passwordExpired', null, "Password Expired", request.locale)
msg = messageSource.getMessage('springSecurity.errors.login.passwordExpired', null, "Password Expired", locale)
} else if (exception instanceof DisabledException) {
msg = messageSource.getMessage('springSecurity.errors.login.disabled', null, "Account Disabled", request.locale)
msg = messageSource.getMessage('springSecurity.errors.login.disabled', null, "Account Disabled", locale)
} else if (exception instanceof LockedException) {
msg = messageSource.getMessage('springSecurity.errors.login.locked', null, "Account Locked", request.locale)
msg = messageSource.getMessage('springSecurity.errors.login.locked', null, "Account Locked", locale)
} else if (exception instanceof SessionAuthenticationException) {
msg = messageSource.getMessage('springSecurity.errors.login.max.sessions.exceeded', null, "Sorry, you have exceeded your maximum number of open sessions.", request.locale)
msg = messageSource.getMessage('springSecurity.errors.login.max.sessions.exceeded', null, "Sorry, you have exceeded your maximum number of open sessions.", locale)
} else {
msg = messageSource.getMessage('springSecurity.errors.login.fail', null, "Authentication Failure", request.locale)
msg = messageSource.getMessage('springSecurity.errors.login.fail', null, "Authentication Failure", locale)
}
}

Expand Down
Loading