Skip to content

Commit

Permalink
Fix language switch on custom forms
Browse files Browse the repository at this point in the history 
- FreeMarkerLoginFormsProvider
  - extract helper methods to build the locale bean (see #createLocaleBean), in order to reduce cyclomatic complexity
  - introduce new logic for custom forms in new method #createCustomFormLocaleUrlBuilder (with separate logic for "../login-actions" urls and other URLs such as authorization endpoint URL)
- AuthorizationEndpoint#buildAuthorizationCodeAuthorizationResponse: add call to processLocaleParam, to make sure that locale switch is also evaluated for custom forms
- LoginActionsService#authenticate: move up the call to processLocaleParam, to support localization of "expired page"
- add test CustomFlowTest#customAuthenticatorLocalization
- Adjust test class ClickThroughAuthenticator to succeed only when "accept" has been clicked (before the check was that "decline" has NOT been clicked) - otherwise locale switch does not work as expected

Closes keycloak#16063

Signed-off-by: Daniel Fesenmeyer <[email protected]>
  • Loading branch information
@danielFesenmeyer
danielFesenmeyer committed Feb 27, 2025
1 parent f7e21af commit 469acc4
Show file tree
Hide file tree
Showing 7 changed files with 243 additions and 76 deletions.
157 changes: 100 additions & 57 deletions services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,63 +503,7 @@ protected void createCommonAttributes(Theme theme, Locale locale, Properties mes
setAttribute(Constants.EXECUTION, execution);

if (realm.isInternationalizationEnabled()) {
UriBuilder b;
if (page != null) {
switch (page) {
case LOGIN:
case LOGIN_USERNAME:
case LOGIN_WEBAUTHN:
case X509_CONFIRM:
b = UriBuilder.fromUri(Urls.realmLoginPage(baseUri, realm.getName()));
break;
case REGISTER:
b = UriBuilder.fromUri(Urls.realmRegisterPage(baseUri, realm.getName()));
break;
case LOGOUT_CONFIRM:
b = UriBuilder.fromUri(Urls.logoutConfirm(baseUri, realm.getName()));
break;
case INFO:
case ERROR:
if (isDetachedAuthenticationSession()) {
FormMessage formMessage = getFirstMessage();
if (formMessage == null) {
throw new IllegalStateException("Not able to create info/error page with detached authentication session as no info/error message available");
}

DetachedInfoStateCookie cookie = new DetachedInfoStateChecker(session, realm).generateAndSetCookie(
formMessage.getMessage(), messageType.toString(), status == null ? null : status.getStatusCode(),
client == null ? null : client.getId(), formMessage.getParameters());

b = UriBuilder.fromUri(Urls.loginActionsDetachedInfo(baseUri, realm.getName()))
.queryParam(DetachedInfoStateChecker.STATE_CHECKER_PARAM, cookie.getRenderedUrlState());
break;
}
default:
b = UriBuilder.fromUri(baseUri).path(uriInfo.getPath());
break;
}
} else {
b = UriBuilder.fromUri(baseUri)
.path(uriInfo.getPath());
}

if (execution != null) {
b.queryParam(Constants.EXECUTION, execution);
}

if (authenticationSession != null && authenticationSession.getAuthNote(Constants.KEY) != null) {
b.queryParam(Constants.KEY, authenticationSession.getAuthNote(Constants.KEY));
} else {
HttpRequest request = session.getContext().getHttpRequest();
MultivaluedMap<String, String> queryParameters = request.getUri().getQueryParameters();

if (queryParameters != null && queryParameters.getFirst(Constants.TOKEN) != null) {
// changing locale should forward the action token
b.queryParam(Constants.TOKEN, queryParameters.getFirst(Constants.TOKEN));
}
}

final var localeBean = new LocaleBean(realm, locale, b, messagesBundle);
final var localeBean = createLocaleBean(locale, messagesBundle, baseUri, page);
attributes.put("locale", localeBean);

lang = localeBean.getCurrentLanguageTag();
Expand All @@ -586,6 +530,105 @@ protected void createCommonAttributes(Theme theme, Locale locale, Properties mes
attributes.put("lang", lang);
}

private LocaleBean createLocaleBean(final Locale locale, final Properties messagesBundle, final URI baseUri,
final LoginFormsPages page) {
final var uriBuilder = page != null ? createBuiltinFormLocaleUrlBuilder(baseUri, page)
: createCustomFormLocaleUrlBuilder(baseUri);
return new LocaleBean(realm, locale, uriBuilder, messagesBundle);
}

private UriBuilder createBuiltinFormLocaleUrlBuilder(final URI baseUri, final LoginFormsPages page) {
UriBuilder b;
switch (page) {
case LOGIN:
case LOGIN_USERNAME:
case LOGIN_WEBAUTHN:
case X509_CONFIRM:
b = UriBuilder.fromUri(Urls.realmLoginPage(baseUri, realm.getName()));
break;
case REGISTER:
b = UriBuilder.fromUri(Urls.realmRegisterPage(baseUri, realm.getName()));
break;
case LOGOUT_CONFIRM:
b = UriBuilder.fromUri(Urls.logoutConfirm(baseUri, realm.getName()));
break;
case INFO:
case ERROR:
if (isDetachedAuthenticationSession()) {
FormMessage formMessage = getFirstMessage();
if (formMessage == null) {
throw new IllegalStateException(
"Not able to create info/error page with detached authentication session as no info/error message available");
}

DetachedInfoStateCookie cookie = new DetachedInfoStateChecker(session, realm).generateAndSetCookie(
formMessage.getMessage(), messageType.toString(),
status == null ? null : status.getStatusCode(),
client == null ? null : client.getId(), formMessage.getParameters());

b = UriBuilder.fromUri(Urls.loginActionsDetachedInfo(baseUri, realm.getName()))
.queryParam(DetachedInfoStateChecker.STATE_CHECKER_PARAM, cookie.getRenderedUrlState());
break;
}
default:
b = UriBuilder.fromUri(baseUri).path(uriInfo.getPath());
break;
}

appendCommonLoginParams(b);

return b;
}

private void appendCommonLoginParams(final UriBuilder b) {
if (execution != null) {
b.queryParam(Constants.EXECUTION, execution);
}

if (authenticationSession != null && authenticationSession.getAuthNote(Constants.KEY) != null) {
b.queryParam(Constants.KEY, authenticationSession.getAuthNote(Constants.KEY));
} else {
HttpRequest request = session.getContext().getHttpRequest();
MultivaluedMap<String, String> queryParameters = request.getUri().getQueryParameters();

if (queryParameters != null && queryParameters.getFirst(Constants.TOKEN) != null) {
// changing locale should forward the action token
b.queryParam(Constants.TOKEN, queryParameters.getFirst(Constants.TOKEN));
}
}
}

private UriBuilder createCustomFormLocaleUrlBuilder(final URI baseUri) {
final var relativeLoginActionsBaseUri =
URI.create(Urls.loginActionsBase(URI.create("/")).build(realm.getName()).getPath());
final var relativeRequestedPathUri = URI.create(uriInfo.getPath());
final var isCurrentPathLoginAction = relativeRequestedPathUri.equals(relativeLoginActionsBaseUri) ||
!relativeLoginActionsBaseUri.relativize(relativeRequestedPathUri).equals(relativeRequestedPathUri);

if (isCurrentPathLoginAction) {
// for login actions, use the requested path and append common login params
final var uriBuilder = UriBuilder.fromUri(baseUri)
.path(relativeRequestedPathUri.getPath());

/*
* Add the access code, if one has been created for the form, and it's not part of a required action.
*/
final var accessCodeExistsAndNotInRequiredAction = accessCode != null && execution == null;
if (accessCodeExistsAndNotInRequiredAction) {
uriBuilder.queryParam(LoginActionsService.SESSION_CODE, accessCode);
}

appendCommonLoginParams(uriBuilder);

return uriBuilder;
} else {
/* for "non-login-actions" requests (authorization endpoint URL), use the requested path and query string */
return UriBuilder.fromUri(baseUri)
.path(uriInfo.getPath())
.replaceQuery(uriInfo.getRequestUri().getQuery());
}
}

/**
* Process FreeMarker template and prepare Response. Some fields are used for rendering also.
*
Expand Down
3 changes: 3 additions & 0 deletions services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,9 @@ private Response buildAuthorizationCodeAuthorizationResponse() {
this.event.event(EventType.LOGIN);
authenticationSession.setAuthNote(Details.AUTH_TYPE, CODE_AUTH_TYPE);

// evaluate the locale param, in order to support locale switch for all login pages (including custom pages)
LocaleUtil.processLocaleParam(session, realm, authenticationSession);

return handleBrowserAuthenticationRequest(authenticationSession, new OIDCLoginProtocol(session, realm, session.getContext().getUri(), headers, event), TokenUtil.hasPrompt(request.getPrompt(), OIDCLoginProtocol.PROMPT_VALUE_NONE), false);
}

Expand Down
12 changes: 10 additions & 2 deletions services/src/main/java/org/keycloak/services/resources/LoginActionsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,16 @@ public Response authenticate(@QueryParam(AUTH_SESSION_ID) String authSessionId,

event.event(EventType.LOGIN);

/*
* evaluate the locale param, in order to support locale switch for all login pages
* (including custom pages and "expired page", which may be shown by SessionCodeChecks done below)
*/
final var requestedClient = realm.getClientByClientId(clientId);
final var currentAuthSession =
new AuthenticationSessionManager(session).getCurrentAuthenticationSession(realm, requestedClient,
tabId);
processLocaleParam(currentAuthSession);

SessionCodeChecks checks = checksForCode(authSessionId, code, execution, clientId, tabId, clientData, AUTHENTICATE_PATH);
if (!checks.verifyActiveAndValidAction(AuthenticationSessionModel.Action.AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
return checks.getResponse();
Expand All @@ -336,8 +346,6 @@ public Response authenticate(@QueryParam(AUTH_SESSION_ID) String authSessionId,
AuthenticationSessionModel authSession = checks.getAuthenticationSession();
boolean actionRequest = checks.isActionRequest();

processLocaleParam(authSession);

return processAuthentication(actionRequest, execution, authSession, null);
}

Expand Down
3 changes: 2 additions & 1 deletion ...suite-providers/src/main/java/org/keycloak/testsuite/forms/ClickThroughAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ public void setRequiredActions(KeycloakSession session, RealmModel realm, UserMo

@Override
public void action(AuthenticationFlowContext context) {
if (context.getHttpRequest().getDecodedFormParameters().containsKey("cancel")) {
final var accepted = context.getHttpRequest().getDecodedFormParameters().containsKey("accept");
if (!accepted) {
authenticate(context);
return;
}
Expand Down
14 changes: 13 additions & 1 deletion ...ration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/AbstractPage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@
import org.keycloak.testsuite.util.oauth.OAuthClient;
import org.openqa.selenium.WebDriver;

import java.util.Objects;

import static org.junit.Assert.assertEquals;

/**
* @author <a href="mailto:[email protected]">Stian Thorgersen</a>
*/
Expand All @@ -42,7 +46,15 @@ public void assertCurrent() {
abstract public boolean isCurrent();

public boolean isCurrent(String expectedTitle) {
return PageUtils.getPageTitle(driver).equals(expectedTitle);
return Objects.equals(getPageTitle(), expectedTitle);
}

public void assertPageTitle(final String expectedTitle) {
assertEquals(expectedTitle, getPageTitle());
}

private String getPageTitle() {
return PageUtils.getPageTitle(driver);
}

public void setDriver(WebDriver driver) {
Expand Down
2 changes: 1 addition & 1 deletion ...uillian/tests/base/src/main/java/org/keycloak/testsuite/pages/TermsAndConditionsPage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
/**
* @author <a href="mailto:[email protected]">Stian Thorgersen</a>
*/
public class TermsAndConditionsPage extends AbstractPage {
public class TermsAndConditionsPage extends LanguageComboboxAwarePage {

@FindBy(id = "kc-accept")
private WebElement submitButton;
Expand Down
Loading

0 comments on commit 469acc4

Please sign in to comment.