From 5a05d2123ee14f36b64b6aac08041ef7623734cf Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 8 Sep 2025 09:05:48 +0200 Subject: [PATCH] Unbounded login_hint parameter Can Corrupt KC_RESTART Cookie closes #40857 Signed-off-by: mposolda --- .../topics/templates/document-attributes.adoc | 2 + .../topics/changes/changes-26_4_0.adoc | 10 ++ .../oidc/OIDCLoginProtocolFactory.java | 63 ++++++++- .../protocol/oidc/OIDCProviderConfig.java | 53 +++++++- .../request/AuthzEndpointRequestParser.java | 57 ++++++--- .../testframework/ui/page/LoginPage.java | 9 ++ ...EndpointRequestParserCustomConfigTest.java | 64 ++++++++++ .../forms/AuthzEndpointRequestParserTest.java | 120 ++++++++++++++++++ .../authz/AuthzEndpointRequestParserTest.java | 47 ------- 9 files changed, 351 insertions(+), 74 deletions(-) create mode 100644 tests/base/src/test/java/org/keycloak/tests/forms/AuthzEndpointRequestParserCustomConfigTest.java create mode 100644 tests/base/src/test/java/org/keycloak/tests/forms/AuthzEndpointRequestParserTest.java delete mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AuthzEndpointRequestParserTest.java diff --git a/docs/documentation/topics/templates/document-attributes.adoc b/docs/documentation/topics/templates/document-attributes.adoc index 3d503401f97..7e52af05335 100644 --- a/docs/documentation/topics/templates/document-attributes.adoc +++ b/docs/documentation/topics/templates/document-attributes.adoc @@ -58,6 +58,8 @@ :apidocs_link: https://www.keycloak.org/docs/{project_version}/api_documentation/ :adminguide_email_name: Configuring email for a realm :adminguide_email_link: {adminguide_link}#_email +:allproviderconfigguide_name: All provider configuration Guide +:allproviderconfigguide_link: https://www.keycloak.org/server/all-provider-config :bootstrapadminrecovery_name: Admin Bootstrap and Recovery :bootstrapadminrecovery_link: https://www.keycloak.org/server/bootstrap-admin-recovery :client_certificate_lookup_link: https://www.keycloak.org/server/reverseproxy#_enabling_client_certificate_lookup diff --git a/docs/documentation/upgrading/topics/changes/changes-26_4_0.adoc b/docs/documentation/upgrading/topics/changes/changes-26_4_0.adoc index 3b43a5783c5..5fa742af002 100644 --- a/docs/documentation/upgrading/topics/changes/changes-26_4_0.adoc +++ b/docs/documentation/upgrading/topics/changes/changes-26_4_0.adoc @@ -176,6 +176,16 @@ The input fields in the login theme for OTP and recovery codes and have been op * The input mode is now `numeric`, which will ease the input on mobile devices. * The auto-complete is set to `one-time-code` to avoid interference with password managers. +=== Maximum length of the parameters in the OIDC authentication request + +When the OIDC authentication request (or OAuth2 authorization request) is sent, there is now limit for the maximum length of every standard OIDC/OAuth2 parameter. The maximum length of each standard parameter is 4000 characters, +which is very big number and can be lowered in the future releases. For now, it is kept big for the backwards compatibility. The only exception is the `login_hint` parameter, which is limited +to the maximum length of 255 characters. This is aligned with the maximum length for the `username` and `email` attributes configured in the default user profile configuration. + +If you want to make those number higher or lower, you can start the server with the option `req-params-default-max-size` for the default maximum length of the standard +OIDC/OAuth2 parameters or you can use something like `req-params-max-size` for one specific parameter. See the `login-protocol` provider configuration +of the link:{allproviderconfigguide_link}[{allproviderconfigguide_name}] for more details. + === UTF-8 management in the email sender Since this release, {project_name} adds a new option `allowutf8` for the realm SMTP configuration (*Allow UTF-8* field inside the *Email* tab in the *Realm settings* section of the Admin Console). diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocolFactory.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocolFactory.java index fbbee59da91..1f825a9c4ed 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocolFactory.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocolFactory.java @@ -23,6 +23,7 @@ import org.keycloak.common.Profile; import org.keycloak.common.constants.KerberosConstants; import org.keycloak.common.constants.ServiceAccountConstants; import org.keycloak.common.util.UriUtils; +import org.keycloak.connections.httpclient.HttpClientProvider; import org.keycloak.events.EventBuilder; import org.keycloak.models.ClientModel; import org.keycloak.models.ClientScopeModel; @@ -47,6 +48,8 @@ import org.keycloak.protocol.oidc.mappers.UserPropertyMapper; import org.keycloak.protocol.oidc.mappers.UserRealmRoleMappingMapper; import org.keycloak.protocol.oidc.mappers.UserSessionNoteMapper; import org.keycloak.protocol.oidc.mappers.SubMapper; +import org.keycloak.provider.ProviderConfigProperty; +import org.keycloak.provider.ProviderConfigurationBuilder; import org.keycloak.representations.IDToken; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.services.ServicesLogger; @@ -54,11 +57,17 @@ import org.keycloak.services.managers.AuthenticationManager; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import static org.keycloak.models.ImpersonationSessionNote.IMPERSONATOR_ID; import static org.keycloak.models.ImpersonationSessionNote.IMPERSONATOR_USERNAME; +import static org.keycloak.protocol.oidc.OIDCProviderConfig.DEFAULT_ADDITIONAL_REQ_PARAMS_FAIL_FAST; +import static org.keycloak.protocol.oidc.OIDCProviderConfig.DEFAULT_ADDITIONAL_REQ_PARAMS_MAX_NUMBER; +import static org.keycloak.protocol.oidc.OIDCProviderConfig.DEFAULT_ADDITIONAL_REQ_PARAMS_MAX_OVERALL_SIZE; +import static org.keycloak.protocol.oidc.OIDCProviderConfig.DEFAULT_ADDITIONAL_REQ_PARAMS_MAX_SIZE; +import static org.keycloak.protocol.oidc.OIDCProviderConfig.DEFAULT_REQ_PARAMS_DEFAULT_MAX_SIZE; /** * @author Bill Burke @@ -115,10 +124,12 @@ public class OIDCLoginProtocolFactory extends AbstractLoginProtocolFactory { public static final String ROLES_SCOPE_CONSENT_TEXT = "${rolesScopeConsentText}"; public static final String ORGANIZATION_SCOPE_CONSENT_TEXT = "${organizationScopeConsentText}"; - public static final String CONFIG_OIDC_REQ_PARAMS_MAX_NUMBER = "add-req-params-max-number"; - public static final String CONFIG_OIDC_REQ_PARAMS_MAX_SIZE = "add-req-params-max-size"; - public static final String CONFIG_OIDC_REQ_PARAMS_MAX_OVERALL_SIZE = "add-req-params-max-overall-size"; - public static final String CONFIG_OIDC_REQ_PARAMS_FAIL_FAST = "add-req-params-fail-fast"; + public static final String CONFIG_OIDC_REQ_PARAMS_DEFAULT_MAX_SIZE = "req-params-default-max-size"; + public static final String CONFIG_OIDC_REQ_PARAMS_MAX_SIZE_PREFIX = "req-params-max-size"; + public static final String CONFIG_OIDC_ADD_REQ_PARAMS_MAX_NUMBER = "add-req-params-max-number"; + public static final String CONFIG_OIDC_ADD_REQ_PARAMS_MAX_SIZE = "add-req-params-max-size"; + public static final String CONFIG_OIDC_ADD_REQ_PARAMS_MAX_OVERALL_SIZE = "add-req-params-max-overall-size"; + public static final String CONFIG_OIDC_ADD_REQ_PARAMS_FAIL_FAST = "add-req-params-fail-fast"; /** * @deprecated To be removed in Keycloak 27 @@ -554,4 +565,48 @@ public class OIDCLoginProtocolFactory extends AbstractLoginProtocolFactory { public int order() { return UI_ORDER; } + + @Override + public List getConfigMetadata() { + return ProviderConfigurationBuilder.create() + .property() + .name(CONFIG_OIDC_REQ_PARAMS_DEFAULT_MAX_SIZE) + .type("int") + .helpText("Maximum default length of the standard OIDC parameter sent to the OIDC authentication request. This applies to most of the standard parameters like for example 'state', 'nonce' etc." + + " The exception is 'login_hint' parameter, which has maximum length of 255 characters.") + .defaultValue(DEFAULT_REQ_PARAMS_DEFAULT_MAX_SIZE) + .add() + .property() + .name(CONFIG_OIDC_REQ_PARAMS_MAX_SIZE_PREFIX + "--" + OIDCLoginProtocol.LOGIN_HINT_PARAM) + .type("int") + .helpText("Maximum length of the standard OIDC authentication request parameter overriden for the specified parameter. Useful if some standard OIDC parameter should have different limit than '" + CONFIG_OIDC_REQ_PARAMS_DEFAULT_MAX_SIZE + + "'. It is needed to add the name of the parameter after this prefix into the configuration. In this example, the '" + OIDCLoginProtocol.LOGIN_HINT_PARAM + "' parameter is used, but this format is supported for any known standard OIDC/OAuth2 parameter.") + .add() + .property() + .name(CONFIG_OIDC_ADD_REQ_PARAMS_MAX_NUMBER) + .type("int") + .helpText("Maximum number of additional request parameters sent to the OIDC authentication request. As 'additional request parameter' is meant some custom parameter not directly treated as standard OIDC/OAuth2 protocol parameter. Additional parameters might be useful for example to add custom claims to the OIDC token (in case that also particular protocol mappers are configured).") + .defaultValue(DEFAULT_ADDITIONAL_REQ_PARAMS_MAX_NUMBER) + .add() + .property() + .name(CONFIG_OIDC_ADD_REQ_PARAMS_MAX_SIZE) + .type("int") + .helpText("Maximum size of single additional request parameter value See '" + CONFIG_OIDC_ADD_REQ_PARAMS_MAX_NUMBER + "' for more details about additional request parameters") + .defaultValue(DEFAULT_ADDITIONAL_REQ_PARAMS_MAX_SIZE) + .add() + .property() + .name(CONFIG_OIDC_ADD_REQ_PARAMS_MAX_OVERALL_SIZE) + .type("int") + .helpText("Maximum size of all additional request parameters values together. See '" + CONFIG_OIDC_ADD_REQ_PARAMS_MAX_NUMBER + "' for more details about additional request parameters") + .defaultValue(DEFAULT_ADDITIONAL_REQ_PARAMS_MAX_OVERALL_SIZE) + .add() + .property() + .name(CONFIG_OIDC_ADD_REQ_PARAMS_FAIL_FAST) + .type("boolean") + .helpText("Whether the fail-fast strategy should be enforced in case if the limit for some standard OIDC parameter or additional OIDC parameter is not met for the parameters sent to the OIDC authentication request." + + " If false, then all additional request parameters to not meet the configuration are silently ignored. If true, an exception will be raised and OIDC authentication request will not be allowed.") + .defaultValue(DEFAULT_ADDITIONAL_REQ_PARAMS_FAIL_FAST) + .add() + .build(); + } } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCProviderConfig.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCProviderConfig.java index b2009dd43d3..f37c0c9aae6 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCProviderConfig.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCProviderConfig.java @@ -1,5 +1,7 @@ package org.keycloak.protocol.oidc; +import java.util.Map; + import org.keycloak.Config; /** @@ -7,6 +9,24 @@ import org.keycloak.Config; */ public class OIDCProviderConfig { + private final Config.Scope config; + + /** + * Maximum default length of the standard OIDC parameter sent to the OIDC authentication request. + */ + public static final int DEFAULT_REQ_PARAMS_DEFAULT_MAX_SIZE = 4000; + + private final int reqParamsDefaultMaxSize; + + /** + * Overriden values for maximum sizes of specified standard OIDC parameters. The value for the specified parameter can be still overriden + * by administrator in the configuration of the {@link OIDCLoginProtocolFactory}. In case that value is not overriden in the configuration or in this map, + * then the value specified by the {@link OIDCLoginProtocolFactory#CONFIG_OIDC_REQ_PARAMS_DEFAULT_MAX_SIZE} is used + */ + private Map DEFAULT_MAX_PARAMS_SIZES = Map.of( + OIDCLoginProtocol.LOGIN_HINT_PARAM, 255 // Aligned with user-profile configuration for username and email + ); + /** * Default value for {@link #additionalReqParamsMaxNumber} if case no configuration property is set. */ @@ -62,10 +82,13 @@ public class OIDCProviderConfig { public OIDCProviderConfig(Config.Scope config) { - this.additionalReqParamsMaxNumber = config.getInt(OIDCLoginProtocolFactory.CONFIG_OIDC_REQ_PARAMS_MAX_NUMBER, DEFAULT_ADDITIONAL_REQ_PARAMS_MAX_NUMBER); - this.additionalReqParamsMaxSize = config.getInt(OIDCLoginProtocolFactory.CONFIG_OIDC_REQ_PARAMS_MAX_SIZE, DEFAULT_ADDITIONAL_REQ_PARAMS_MAX_SIZE); - this.additionalReqParamsMaxOverallSize = config.getInt(OIDCLoginProtocolFactory.CONFIG_OIDC_REQ_PARAMS_MAX_OVERALL_SIZE, DEFAULT_ADDITIONAL_REQ_PARAMS_MAX_OVERALL_SIZE); - this.additionalReqParamsFailFast = config.getBoolean(OIDCLoginProtocolFactory.CONFIG_OIDC_REQ_PARAMS_FAIL_FAST, DEFAULT_ADDITIONAL_REQ_PARAMS_FAIL_FAST); + this.config = config; + + this.reqParamsDefaultMaxSize = config.getInt(OIDCLoginProtocolFactory.CONFIG_OIDC_REQ_PARAMS_DEFAULT_MAX_SIZE, DEFAULT_REQ_PARAMS_DEFAULT_MAX_SIZE); + this.additionalReqParamsMaxNumber = config.getInt(OIDCLoginProtocolFactory.CONFIG_OIDC_ADD_REQ_PARAMS_MAX_NUMBER, DEFAULT_ADDITIONAL_REQ_PARAMS_MAX_NUMBER); + this.additionalReqParamsMaxSize = config.getInt(OIDCLoginProtocolFactory.CONFIG_OIDC_ADD_REQ_PARAMS_MAX_SIZE, DEFAULT_ADDITIONAL_REQ_PARAMS_MAX_SIZE); + this.additionalReqParamsMaxOverallSize = config.getInt(OIDCLoginProtocolFactory.CONFIG_OIDC_ADD_REQ_PARAMS_MAX_OVERALL_SIZE, DEFAULT_ADDITIONAL_REQ_PARAMS_MAX_OVERALL_SIZE); + this.additionalReqParamsFailFast = config.getBoolean(OIDCLoginProtocolFactory.CONFIG_OIDC_ADD_REQ_PARAMS_FAIL_FAST, DEFAULT_ADDITIONAL_REQ_PARAMS_FAIL_FAST); this.allowMultipleAudiencesForJwtClientAuthentication = config.getBoolean(OIDCLoginProtocolFactory.CONFIG_OIDC_ALLOW_MULTIPLE_AUDIENCES_FOR_JWT_CLIENT_AUTHENTICATION, DEFAULT_ALLOW_MULTIPLE_AUDIENCES_FOR_JWT_CLIENT_AUTHENTICATION); } @@ -89,4 +112,26 @@ public class OIDCProviderConfig { public boolean isAllowMultipleAudiencesForJwtClientAuthentication() { return allowMultipleAudiencesForJwtClientAuthentication; } + + /** + * @param paramName Parameter name. Expected to be one of the known OIDC parameters + * + * @return maximum length for the specified OIDC parameter + */ + public int getMaxLengthForTheParameter(String paramName) { + // Configured value for the particular OIDC parameter + Integer paramMaxSize = config.getInt(OIDCLoginProtocolFactory.CONFIG_OIDC_REQ_PARAMS_MAX_SIZE_PREFIX + "--" + paramName); + + // Stick to default. See if we have default value overriden + if (paramMaxSize == null) { + paramMaxSize = DEFAULT_MAX_PARAMS_SIZES.get(paramName); + } + + // Fallback to default for all standard OIDC parameters + if (paramMaxSize == null) { + paramMaxSize = reqParamsDefaultMaxSize; + } + + return paramMaxSize; + } } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/request/AuthzEndpointRequestParser.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/request/AuthzEndpointRequestParser.java index f75df6198ac..5863bc069d0 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/request/AuthzEndpointRequestParser.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/request/AuthzEndpointRequestParser.java @@ -60,6 +60,7 @@ public abstract class AuthzEndpointRequestParser { private static final Logger logger = Logger.getLogger(AuthzEndpointRequestParser.class); + protected final OIDCProviderConfig config; protected final int additionalReqParamsMaxNumber; protected final int additionalReqParamsMaxSize; protected final boolean additionalReqParamsFailFast; @@ -104,7 +105,7 @@ public abstract class AuthzEndpointRequestParser { protected AuthzEndpointRequestParser(KeycloakSession keycloakSession) { OIDCLoginProtocol loginProtocol = (OIDCLoginProtocol) keycloakSession.getProvider(LoginProtocol.class, OIDCLoginProtocol.LOGIN_PROTOCOL); - OIDCProviderConfig config = loginProtocol.getConfig(); + this.config = loginProtocol.getConfig(); this.additionalReqParamsMaxNumber = config.getAdditionalReqParamsMaxNumber(); this.additionalReqParamsMaxSize = config.getAdditionalReqParamsMaxSize(); this.additionalReqParamsFailFast = config.isAdditionalReqParamsFailFast(); @@ -112,7 +113,7 @@ public abstract class AuthzEndpointRequestParser { } public void parseRequest(AuthorizationEndpointRequest request) { - String clientId = getParameter(OIDCLoginProtocol.CLIENT_ID_PARAM); + String clientId = getAndValidateParameter(OIDCLoginProtocol.CLIENT_ID_PARAM); if (clientId != null && request.clientId != null && !request.clientId.equals(clientId)) { throw new IllegalArgumentException("The client_id parameter doesn't match the one from OIDC 'request' or 'request_uri'"); } @@ -120,32 +121,32 @@ public abstract class AuthzEndpointRequestParser { request.clientId = clientId; } - String responseType = getParameter(OIDCLoginProtocol.RESPONSE_TYPE_PARAM); + String responseType = getAndValidateParameter(OIDCLoginProtocol.RESPONSE_TYPE_PARAM); validateResponseTypeParameter(responseType, request); if (responseType != null) { request.responseType = responseType; } - request.responseMode = replaceIfNotNull(request.responseMode, getParameter(OIDCLoginProtocol.RESPONSE_MODE_PARAM)); - request.redirectUriParam = replaceIfNotNull(request.redirectUriParam, getParameter(OIDCLoginProtocol.REDIRECT_URI_PARAM)); - request.state = replaceIfNotNull(request.state, getParameter(OIDCLoginProtocol.STATE_PARAM)); - request.scope = replaceIfNotNull(request.scope, getParameter(OIDCLoginProtocol.SCOPE_PARAM)); - request.loginHint = replaceIfNotNull(request.loginHint, getParameter(OIDCLoginProtocol.LOGIN_HINT_PARAM)); - request.prompt = replaceIfNotNull(request.prompt, getParameter(OIDCLoginProtocol.PROMPT_PARAM)); - request.idpHint = replaceIfNotNull(request.idpHint, getParameter(AdapterConstants.KC_IDP_HINT)); - request.action = replaceIfNotNull(request.action, getParameter(Constants.KC_ACTION)); - request.nonce = replaceIfNotNull(request.nonce, getParameter(OIDCLoginProtocol.NONCE_PARAM)); + request.responseMode = replaceIfNotNull(request.responseMode, getAndValidateParameter(OIDCLoginProtocol.RESPONSE_MODE_PARAM)); + request.redirectUriParam = replaceIfNotNull(request.redirectUriParam, getAndValidateParameter(OIDCLoginProtocol.REDIRECT_URI_PARAM)); + request.state = replaceIfNotNull(request.state, getAndValidateParameter(OIDCLoginProtocol.STATE_PARAM)); + request.scope = replaceIfNotNull(request.scope, getAndValidateParameter(OIDCLoginProtocol.SCOPE_PARAM)); + request.loginHint = replaceIfNotNull(request.loginHint, getAndValidateParameter(OIDCLoginProtocol.LOGIN_HINT_PARAM)); + request.prompt = replaceIfNotNull(request.prompt, getAndValidateParameter(OIDCLoginProtocol.PROMPT_PARAM)); + request.idpHint = replaceIfNotNull(request.idpHint, getAndValidateParameter(AdapterConstants.KC_IDP_HINT)); + request.action = replaceIfNotNull(request.action, getAndValidateParameter(Constants.KC_ACTION)); + request.nonce = replaceIfNotNull(request.nonce, getAndValidateParameter(OIDCLoginProtocol.NONCE_PARAM)); request.maxAge = replaceIfNotNull(request.maxAge, getIntParameter(OIDCLoginProtocol.MAX_AGE_PARAM)); - request.claims = replaceIfNotNull(request.claims, getParameter(OIDCLoginProtocol.CLAIMS_PARAM)); - request.acr = replaceIfNotNull(request.acr, getParameter(OIDCLoginProtocol.ACR_PARAM)); - request.display = replaceIfNotNull(request.display, getParameter(OAuth2Constants.DISPLAY)); - request.uiLocales = replaceIfNotNull(request.uiLocales, getParameter(OAuth2Constants.UI_LOCALES_PARAM)); + request.claims = replaceIfNotNull(request.claims, getAndValidateParameter(OIDCLoginProtocol.CLAIMS_PARAM)); + request.acr = replaceIfNotNull(request.acr, getAndValidateParameter(OIDCLoginProtocol.ACR_PARAM)); + request.display = replaceIfNotNull(request.display, getAndValidateParameter(OAuth2Constants.DISPLAY)); + request.uiLocales = replaceIfNotNull(request.uiLocales, getAndValidateParameter(OAuth2Constants.UI_LOCALES_PARAM)); // https://tools.ietf.org/html/rfc7636#section-6.1 - request.codeChallenge = replaceIfNotNull(request.codeChallenge, getParameter(OIDCLoginProtocol.CODE_CHALLENGE_PARAM)); - request.codeChallengeMethod = replaceIfNotNull(request.codeChallengeMethod, getParameter(OIDCLoginProtocol.CODE_CHALLENGE_METHOD_PARAM)); + request.codeChallenge = replaceIfNotNull(request.codeChallenge, getAndValidateParameter(OIDCLoginProtocol.CODE_CHALLENGE_PARAM)); + request.codeChallengeMethod = replaceIfNotNull(request.codeChallengeMethod, getAndValidateParameter(OIDCLoginProtocol.CODE_CHALLENGE_METHOD_PARAM)); - request.dpopJkt = replaceIfNotNull(request.dpopJkt, getParameter(OIDCLoginProtocol.DPOP_JKT)); + request.dpopJkt = replaceIfNotNull(request.dpopJkt, getAndValidateParameter(OIDCLoginProtocol.DPOP_JKT)); extractAdditionalReqParams(request.additionalReqParams); } @@ -220,6 +221,24 @@ public abstract class AuthzEndpointRequestParser { return newVal==null ? previousVal : newVal; } + protected String getAndValidateParameter(String paramName) { + String paramValue = getParameter(paramName); + + if (paramValue != null) { + int maxLength = config.getMaxLengthForTheParameter(paramName); + if (paramValue.length() > maxLength) { + logger.warnf("The size of OIDC parameter '%s' size is longer (%d) than allowed (%d). %s", paramName, paramValue.length(), maxLength, additionalReqParamsFailFast ? "Request not allowed." : "Ignoring the parameter."); + if (additionalReqParamsFailFast) { + throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "The size of OIDC parameter '" + paramName + "' is longer than allowed.", Response.Status.BAD_REQUEST); + } else { + return null; + } + } + } + + return paramValue; + } + protected abstract String getParameter(String paramName); protected abstract Integer getIntParameter(String paramName); diff --git a/test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/LoginPage.java b/test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/LoginPage.java index 5b02e7f05f2..8bd0b2fcf70 100644 --- a/test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/LoginPage.java +++ b/test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/LoginPage.java @@ -1,6 +1,7 @@ package org.keycloak.testframework.ui.page; import org.openqa.selenium.By; +import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.FindBy; @@ -43,4 +44,12 @@ public class LoginPage extends AbstractPage { public String getExpectedPageId() { return "login-login"; } + + public String getUsername() { + return usernameInput.getAttribute("value"); + } + + public void clearUsernameInput() { + usernameInput.clear(); + } } diff --git a/tests/base/src/test/java/org/keycloak/tests/forms/AuthzEndpointRequestParserCustomConfigTest.java b/tests/base/src/test/java/org/keycloak/tests/forms/AuthzEndpointRequestParserCustomConfigTest.java new file mode 100644 index 00000000000..9ee71a159e4 --- /dev/null +++ b/tests/base/src/test/java/org/keycloak/tests/forms/AuthzEndpointRequestParserCustomConfigTest.java @@ -0,0 +1,64 @@ +package org.keycloak.tests.forms; + +import org.junit.jupiter.api.Test; +import org.keycloak.common.util.SecretGenerator; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; +import org.keycloak.protocol.oidc.OIDCLoginProtocolFactory; +import org.keycloak.testframework.annotations.KeycloakIntegrationTest; +import org.keycloak.testframework.server.KeycloakServerConfig; +import org.keycloak.testframework.server.KeycloakServerConfigBuilder; + +/** + * Custom configuration of OIDC login protocol factory with some overriden config values + * + * @author Marek Posolda + */ +@KeycloakIntegrationTest(config = AuthzEndpointRequestParserCustomConfigTest.AuthzEndpointRequestParserConfig.class) +public class AuthzEndpointRequestParserCustomConfigTest extends AuthzEndpointRequestParserTest { + + @Test + @Override + public void testParamsLength() { + // Login hint with length 100 allowed, state with length 100 allowed + String loginHint100 = SecretGenerator.getInstance().randomString(100); + String state100 = SecretGenerator.getInstance().randomString(100); + oauth.loginForm() + .loginHint(loginHint100) + .state(state100) + .open(); + assertLogin(loginHint100, state100); + + // Login hint with length 200 not allowed, state with length 200 allowed + String loginHint200 = SecretGenerator.getInstance().randomString(200); + String state200 = SecretGenerator.getInstance().randomString(200); + oauth.loginForm() + .loginHint(loginHint200) + .state(state200) + .open(); + assertLogin("", state200); + + // state with length 2100 allowed + String state2100 = SecretGenerator.getInstance().randomString(2100); + oauth.loginForm() + .state(state2100) + .open(); + assertLogin("", state2100); + + // State with length 3100 not allowed + String state3100 = SecretGenerator.getInstance().randomString(3100); + oauth.loginForm() + .state(state3100) + .open(); + assertLogin("", null); + } + + public static class AuthzEndpointRequestParserConfig implements KeycloakServerConfig { + + @Override + public KeycloakServerConfigBuilder configure(KeycloakServerConfigBuilder config) { + return config + .option("spi-login-protocol--" + OIDCLoginProtocol.LOGIN_PROTOCOL + "--" + OIDCLoginProtocolFactory.CONFIG_OIDC_REQ_PARAMS_DEFAULT_MAX_SIZE, "3000") + .option("spi-login-protocol--" + OIDCLoginProtocol.LOGIN_PROTOCOL + "--" + OIDCLoginProtocolFactory.CONFIG_OIDC_REQ_PARAMS_MAX_SIZE_PREFIX + "--login_hint", "100"); + } + } +} diff --git a/tests/base/src/test/java/org/keycloak/tests/forms/AuthzEndpointRequestParserTest.java b/tests/base/src/test/java/org/keycloak/tests/forms/AuthzEndpointRequestParserTest.java new file mode 100644 index 00000000000..0a80de2b4ee --- /dev/null +++ b/tests/base/src/test/java/org/keycloak/tests/forms/AuthzEndpointRequestParserTest.java @@ -0,0 +1,120 @@ +package org.keycloak.tests.forms; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestMethodOrder; +import org.keycloak.common.util.SecretGenerator; +import org.keycloak.testframework.annotations.InjectRealm; +import org.keycloak.testframework.annotations.InjectUser; +import org.keycloak.testframework.annotations.KeycloakIntegrationTest; +import org.keycloak.testframework.oauth.OAuthClient; +import org.keycloak.testframework.oauth.annotations.InjectOAuthClient; +import org.keycloak.testframework.realm.ManagedRealm; +import org.keycloak.testframework.realm.ManagedUser; +import org.keycloak.testframework.realm.UserConfig; +import org.keycloak.testframework.realm.UserConfigBuilder; +import org.keycloak.testframework.ui.annotations.InjectPage; +import org.keycloak.testframework.ui.annotations.InjectWebDriver; +import org.keycloak.testframework.ui.page.LoginPage; +import org.keycloak.testsuite.util.AccountHelper; +import org.openqa.selenium.WebDriver; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Test for default configuration of OIDC login protocol factory + */ +@KeycloakIntegrationTest +@TestMethodOrder(MethodOrderer.MethodName.class) +public class AuthzEndpointRequestParserTest { + + @InjectWebDriver + WebDriver driver; + + @InjectRealm + ManagedRealm realm; + + @InjectUser(ref = "test-user", config = TestUserConfig.class) + ManagedUser managedUser; + + @InjectOAuthClient + OAuthClient oauth; + + @InjectPage + LoginPage loginPage; + + @Test + public void testAuthenticationBackwardsCompatible() { + SecretGenerator.getInstance().randomString(2000 + 1); + + oauth.loginForm() + .param("paramkey1_too_long", SecretGenerator.getInstance().randomString(2000 + 1)) + .param("paramkey2", "paramvalue2") + .param("paramkey3", "paramvalue3") + .param("paramkey4", "paramvalue4") + .param("paramkey5", "paramvalue5") + .param("paramkey6_too_many", "paramvalue6").open(); + loginPage.assertCurrent(); + + } + + @Test + public void testParamsLength() { + // Login hint with length 200 allowed, state with length 200 allowed + String loginHint200 = SecretGenerator.getInstance().randomString(200); + String state200 = SecretGenerator.getInstance().randomString(200); + oauth.loginForm() + .loginHint(loginHint200) + .state(state200) + .open(); + assertLogin(loginHint200, state200); + + // Login hint with length 500 not allowed, state with length 500 allowed + String loginHint500 = SecretGenerator.getInstance().randomString(500); + String state500 = SecretGenerator.getInstance().randomString(500); + oauth.loginForm() + .loginHint(loginHint500) + .state(state500) + .open(); + assertLogin("", state500); + + // state with length 4100 not allowed + String state4100 = SecretGenerator.getInstance().randomString(4100); + oauth.loginForm() + .state(state4100) + .open(); + assertLogin("", null); + } + + protected void assertLogin(String loginHintExpected, String stateExpected) { + loginPage.assertCurrent(); + Assertions.assertEquals(loginHintExpected, loginPage.getUsername()); + loginPage.clearUsernameInput(); + loginPage.fillLogin("test-user", "password"); + loginPage.submit(); + + assertTrue(driver.getPageSource().contains("Happy days")); + // String currentUrl = driver.getCurrentUrl(); + String state = oauth.parseLoginResponse().getState(); + Assertions.assertEquals(stateExpected, state); + + AccountHelper.logout(realm.admin(), "test-user"); + } + + + private static class TestUserConfig implements UserConfig { + + @Override + public UserConfigBuilder configure(UserConfigBuilder user) { + user.username("test-user"); + user.password("password"); + user.name("My", "Test"); + user.email("test@email.org"); + user.emailVerified(true); + + return user; + } + } + +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AuthzEndpointRequestParserTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AuthzEndpointRequestParserTest.java deleted file mode 100644 index 95b4a7c05e9..00000000000 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AuthzEndpointRequestParserTest.java +++ /dev/null @@ -1,47 +0,0 @@ -package org.keycloak.testsuite.authz; - -import jakarta.ws.rs.client.Client; -import jakarta.ws.rs.core.Response; -import org.apache.commons.lang.RandomStringUtils; -import org.junit.Test; -import org.keycloak.representations.idm.RealmRepresentation; -import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; -import org.keycloak.testsuite.util.AdminClientUtil; -import org.keycloak.testsuite.util.Matchers; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; - -public class AuthzEndpointRequestParserTest extends AbstractTestRealmKeycloakTest { - - @Override - public void configureTestRealm(RealmRepresentation testRealm) { - } - - @Test - public void test_authentication_backwards_compatible() { - - try (Client client = AdminClientUtil.createResteasyClient()) { - - String loginUrl = oauth.loginForm() - .param("paramkey1_too_long", RandomStringUtils.random(2000 + 1)) - .param("paramkey2", "paramvalue2") - .param("paramkey3", "paramvalue3") - .param("paramkey4", "paramvalue4") - .param("paramkey5", "paramvalue5") - .param("paramkey6_too_many", "paramvalue6").build(); - - try (Response response = client.target(loginUrl).request().get()) { - - assertThat(response.getStatus(), is(equalTo(200))); - assertThat(response, Matchers.body(containsString("Sign in"))); - - } - - } - - } - -}