From 3497943f9910c16cc5ad626cf8d596434a70cd2e Mon Sep 17 00:00:00 2001 From: Johannes Knutsen Date: Mon, 19 May 2025 18:51:18 +0200 Subject: [PATCH] Add IdP configuration to disable forwarding of the acr_values parameter Closes #39813 Signed-off-by: Johannes Knutsen --- .../admin/messages/messages_en.properties | 2 ++ .../add/ExtendedNonDiscoverySettings.tsx | 1 + .../oidc/AbstractOAuth2IdentityProvider.java | 8 +++-- .../oidc/OAuth2IdentityProviderConfig.java | 9 +++++ .../broker/KcOidcBrokerAcrParameterTest.java | 33 +++++++++++++++++-- 5 files changed, 48 insertions(+), 5 deletions(-) diff --git a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties index 0c4056691dd8..6dd4d2b3619d 100644 --- a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties +++ b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties @@ -1450,6 +1450,8 @@ byConfiguration=By configuration usersAdded_other={{count}} users added to the group userFedUnlinkUsersConfirmTitle=Unlink all users? passCurrentLocale=Pass current locale +passAcrValues=Pass acr_values +passAcrValuesHelp=Pass the current acr_values query parameter on to the identity provider. realmNameField=Realm name roleCreated=Role created socialProfileJSONFieldPath=Social Profile JSON Field Path diff --git a/js/apps/admin-ui/src/identity-providers/add/ExtendedNonDiscoverySettings.tsx b/js/apps/admin-ui/src/identity-providers/add/ExtendedNonDiscoverySettings.tsx index b26dac8b6d48..393014d44a40 100644 --- a/js/apps/admin-ui/src/identity-providers/add/ExtendedNonDiscoverySettings.tsx +++ b/js/apps/admin-ui/src/identity-providers/add/ExtendedNonDiscoverySettings.tsx @@ -42,6 +42,7 @@ export const ExtendedNonDiscoverySettings = () => { + forwardParameters = Arrays.asList(forwardParameterConfig.split("\\s*,\\s*")); diff --git a/services/src/main/java/org/keycloak/broker/oidc/OAuth2IdentityProviderConfig.java b/services/src/main/java/org/keycloak/broker/oidc/OAuth2IdentityProviderConfig.java index ad03d43e0cc9..fac92ab11888 100644 --- a/services/src/main/java/org/keycloak/broker/oidc/OAuth2IdentityProviderConfig.java +++ b/services/src/main/java/org/keycloak/broker/oidc/OAuth2IdentityProviderConfig.java @@ -36,6 +36,7 @@ public class OAuth2IdentityProviderConfig extends IdentityProviderModel { public static final String PKCE_METHOD = "pkceMethod"; public static final String TOKEN_ENDPOINT_URL = "tokenUrl"; public static final String TOKEN_INTROSPECTION_URL = "tokenIntrospectionUrl"; + public static final String FORWARD_ACR_VALUES = "forwardAcrValues"; public static final String JWT_X509_HEADERS_ENABLED = "jwtX509HeadersEnabled"; @@ -143,6 +144,14 @@ public void setForwardParameters(String forwardParameters) { getConfig().put("forwardParameters", forwardParameters); } + public boolean isForwardAcrValues() { + return Boolean.parseBoolean(getConfig().getOrDefault(FORWARD_ACR_VALUES, "false")); + } + + public void setForwardAcrValues(boolean forwardAcrValues) { + getConfig().put(FORWARD_ACR_VALUES, String.valueOf(forwardAcrValues)); + } + public boolean isPkceEnabled() { return Boolean.parseBoolean(getConfig().getOrDefault(PKCE_ENABLED, "false")); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerAcrParameterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerAcrParameterTest.java index 9feb2636c89e..150dcbd36099 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerAcrParameterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerAcrParameterTest.java @@ -1,6 +1,9 @@ package org.keycloak.testsuite.broker; +import org.junit.Test; +import org.keycloak.admin.client.resource.IdentityProviderResource; import org.keycloak.admin.client.resource.UsersResource; +import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.Assert; @@ -18,8 +21,29 @@ protected BrokerConfiguration getBrokerConfiguration() { return KcOidcBrokerConfiguration.INSTANCE; } + @Test + public void testLogInAsUserInIDPWithAcrValues() { + // Forward acr_values = true + IdentityProviderResource idpRes = adminClient + .realm(bc.consumerRealmName()) + .identityProviders() + .get(BrokerTestConstants.IDP_OIDC_ALIAS); + IdentityProviderRepresentation idpRep = idpRes.toRepresentation(); + OIDCIdentityProviderConfigRep cfg = new OIDCIdentityProviderConfigRep(idpRep); + cfg.setForwardAcrValues(true); + idpRes.update(idpRep); + + assertValidLogin(true); + + testSingleLogout(); + } + @Override protected void loginUser() { + assertValidLogin(false); + } + + private void assertValidLogin(boolean expectHasAcrValues) { oauth.clientId("broker-app"); loginPage.open(bc.consumerRealmName()); @@ -33,8 +57,13 @@ protected void loginUser() { Assert.assertTrue("Driver should be on the provider realm page right now", driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/")); - Assert.assertTrue(ACR_VALUES + "=" + ACR_3 + " should be part of the url", - driver.getCurrentUrl().contains(ACR_VALUES + "=" + ACR_3)); + if (expectHasAcrValues) { + Assert.assertTrue(ACR_VALUES + "=" + ACR_3 + " SHOULD be part of the url", + driver.getCurrentUrl().contains(ACR_VALUES + "=" + ACR_3)); + } else { + Assert.assertFalse(ACR_VALUES + "=" + ACR_3 + " SHOULD NOT be part of the url", + driver.getCurrentUrl().contains(ACR_VALUES + "=" + ACR_3)); + } log.debug("Logging in"); loginPage.login(bc.getUserLogin(), bc.getUserPassword());