Fixing encoding of forwarded parameters

pedroigor · web-flow · commit 5d6718354c9b · 2025-11-18T09:58:05.000+01:00
Closes #44125 Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
diff --git a/services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java b/services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java
@@ -528,8 +528,6 @@ protected UriBuilder createAuthorizationUrl(AuthenticationRequest request) {
             uriBuilder.queryParam(OAuth2Constants.PROMPT, prompt);
         }
 
-        setForwardParameters(authenticationSession, uriBuilder);
-
         if (getConfig().isPkceEnabled()) {
             String codeVerifier = PkceUtils.generateCodeVerifier();
             String codeChallengeMethod = getConfig().getPkceMethod();
@@ -541,26 +539,35 @@ protected UriBuilder createAuthorizationUrl(AuthenticationRequest request) {
             uriBuilder.queryParam(OAuth2Constants.CODE_CHALLENGE_METHOD, codeChallengeMethod);
         }
 
+        appendForwardedParameters(authenticationSession, uriBuilder);
+
         return uriBuilder;
     }
 
-    private void setForwardParameters(AuthenticationSessionModel authenticationSession, UriBuilder uriBuilder) {
+    private void appendForwardedParameters(AuthenticationSessionModel authenticationSession, UriBuilder uriBuilder) {
         C config = getConfig();
         String forwardParameterConfig = config.getForwardParameters() != null ? config.getForwardParameters(): OAuth2Constants.ACR_VALUES;
+        List<String> parameterNames = List.of(forwardParameterConfig.split("\\s*,\\s*"));
+        StringBuilder query = new StringBuilder(uriBuilder.build().getRawQuery());
 
-        for (String forwardParameter: List.of(forwardParameterConfig.split("\\s*,\\s*"))) {
-            String name = AuthorizationEndpoint.LOGIN_SESSION_NOTE_ADDITIONAL_REQ_PARAMS_PREFIX + forwardParameter.trim();
-            String parameter = authenticationSession.getClientNote(name);
+        for (String name: parameterNames) {
+            String noteKey = AuthorizationEndpoint.LOGIN_SESSION_NOTE_ADDITIONAL_REQ_PARAMS_PREFIX + name.trim();
+            String value = authenticationSession.getClientNote(noteKey);
 
-            if (parameter == null) {
+            if (value == null) {
                 // try a value set as a client note
-                parameter = authenticationSession.getClientNote(forwardParameter);
+                value = authenticationSession.getClientNote(name);
             }
 
-            if (parameter != null && !parameter.isEmpty()) {
-                uriBuilder.queryParam(forwardParameter, URLEncoder.encode(parameter, StandardCharsets.UTF_8));
+            if (value != null && !value.isEmpty()) {
+                if (!query.isEmpty()) {
+                    query.append("&");
+                }
+                query.append(name).append("=").append(URLEncoder.encode(value, StandardCharsets.UTF_8));
             }
         }
+
+        uriBuilder.replaceQuery(query.toString());
     }
 
     /**
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerParameterForwardTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerParameterForwardTest.java
@@ -50,11 +50,11 @@ protected void loginUser() {
         oauth.clientId("broker-app");
         loginPage.open(bc.consumerRealmName());
 
-        String claimsValue = "{\"userinfo\":{\"http://itsme.services/v2/claim/BENationalNumber\":null}}";
+        String claimsValue = "{\"userinfo\":{\"http://itsme.services/v2/claim/BENationalNumber\":null,\"spaced_value\":\"with space\"}}";
         String urlEncodedClaims = URLEncoder.encode(claimsValue, StandardCharsets.UTF_8);
         String forwardedEncodedParam = "forwarded_encoded";
         String forwardedEncodedParamValue = "encoded value";
-        String forwardedEncodedParamvalueEncoded = URLEncoder.encode(forwardedEncodedParamValue, StandardCharsets.UTF_8);
+        String forwardedEncodedParamValueEncoded = URLEncoder.encode(forwardedEncodedParamValue, StandardCharsets.UTF_8);
         String queryString = "&" + FORWARDED_PARAMETER + "=" + FORWARDED_PARAMETER_VALUE + "&" + PARAMETER_NOT_FORWARDED + "=" + "value"
                 + "&" + OAuth2Constants.ACR_VALUES + "=" + "phr"
                 + "&" + OIDCLoginProtocol.CLAIMS_PARAM + "=" + urlEncodedClaims
@@ -76,7 +76,7 @@ protected void loginUser() {
         assertThat(OIDCLoginProtocol.CLAIMS_PARAM + "=" + urlEncodedClaims + " should be part of the url",
                 driver.getCurrentUrl(), containsString(OIDCLoginProtocol.CLAIMS_PARAM + "=" + urlEncodedClaims));
         assertThat(forwardedEncodedParam + "=" + forwardedEncodedParamValue +  "should be part of the url",
-                driver.getCurrentUrl(), containsString(forwardedEncodedParam + "=" + URLEncoder.encode(forwardedEncodedParamvalueEncoded, StandardCharsets.UTF_8)));
+                driver.getCurrentUrl(), containsString(forwardedEncodedParam + "=" + forwardedEncodedParamValueEncoded));
         assertThat("\"" + PARAMETER_NOT_SET + "\"" + " should NOT be part of the url",
                 driver.getCurrentUrl(), not(containsString(PARAMETER_NOT_SET)));
 
