Revise PAR request object parameter handlig (#34352)

We now store the original parameter value as-is, in case only a single parameter value is provided. In case multiple parameter values are provided
for the same parameter, we only retain the first parameter.
This ensures that the original value is retained. Previously the value list from the
`decodedFormParameters` `MultivaluedMap` was converted to a String while replacing '[' and ']'
with an empty string, which corrupted the original parameter values stored.

Fixes #34352

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>

services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/ParEndpoint.java

@@ -17,6 +17,7 @@
 
 package org.keycloak.protocol.oidc.par.endpoints;
 
+import jakarta.ws.rs.core.MultivaluedMap;
 import org.keycloak.http.HttpRequest;
 import org.keycloak.OAuthErrorException;
 import org.keycloak.common.Profile;
@@ -46,6 +47,7 @@ import jakarta.ws.rs.core.UriBuilder;
 import static org.keycloak.protocol.oidc.OIDCLoginProtocol.REQUEST_URI_PARAM;
 
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 
@@ -88,12 +90,14 @@ public class ParEndpoint extends AbstractParEndpoint {
         checkRealm();
         authorizeClient();
 
-        if (httpRequest.getDecodedFormParameters().containsKey(REQUEST_URI_PARAM)) {
+        MultivaluedMap<String, String> decodedFormParameters = httpRequest.getDecodedFormParameters();
+
+        if (decodedFormParameters.containsKey(REQUEST_URI_PARAM)) {
             throw throwErrorResponseException(OAuthErrorException.INVALID_REQUEST, "It is not allowed to include request_uri to PAR.", Response.Status.BAD_REQUEST);
         }
 
         try {
-            authorizationRequest = ParEndpointRequestParserProcessor.parseRequest(event, session, client, httpRequest.getDecodedFormParameters());
+            authorizationRequest = ParEndpointRequestParserProcessor.parseRequest(event, session, client, decodedFormParameters);
         } catch (Exception e) {
             throw throwErrorResponseException(OAuthErrorException.INVALID_REQUEST_OBJECT, e.getMessage(), Response.Status.BAD_REQUEST);
         }
@@ -138,7 +142,7 @@ public class ParEndpoint extends AbstractParEndpoint {
         }
 
         try {
-            session.clientPolicy().triggerOnEvent(new PushedAuthorizationRequestContext(authorizationRequest, httpRequest.getDecodedFormParameters()));
+            session.clientPolicy().triggerOnEvent(new PushedAuthorizationRequestContext(authorizationRequest, decodedFormParameters));
         } catch (ClientPolicyException cpe) {
             throw throwErrorResponseException(cpe.getError(), cpe.getErrorDetail(), Response.Status.BAD_REQUEST);
         }
@@ -150,11 +154,8 @@ public class ParEndpoint extends AbstractParEndpoint {
 
         int expiresIn = realm.getParPolicy().getRequestUriLifespan();
 
-        httpRequest.getDecodedFormParameters().forEach((k, v) -> {
+        flattenDecodedFormParametersToParamsMap(decodedFormParameters, params);
-                // PAR store only accepts Map so that MultivaluedMap needs to be converted to Map.
+
-                String singleValue = String.valueOf(v).replace("[", "").replace("]", "");
-                params.put(k, singleValue);
-            });
         params.put(PAR_CREATED_TIME, String.valueOf(System.currentTimeMillis()));
 
         SingleUseObjectProvider singleUseStore = session.singleUseObjects();
@@ -168,4 +169,30 @@ public class ParEndpoint extends AbstractParEndpoint {
                 .type(MediaType.APPLICATION_JSON_TYPE));
     }
 
+    /**
+     * Flattens the given decodedFormParameters MultivaluedMap to a plain Map.
+     * Rationale: The SingleUseObjectProvider used as store for PARs only accepts Map so that MultivaluedMap needs to be converted to Map.
+     * @param decodedFormParameters form parameters sent in request body
+     * @param params target parameter Map
+     */
+    public static void flattenDecodedFormParametersToParamsMap(
+            MultivaluedMap<String, String> decodedFormParameters,
+            Map<String, String> params) {
+
+        for (var parameterEntry : decodedFormParameters.entrySet()) {
+            String parameterName = parameterEntry.getKey();
+            List<String> parameterValues = parameterEntry.getValue();
+
+            if (parameterValues.isEmpty()) {
+                // We emit the empty parameter as a marker, but only if it does not exist yet. This prevents "accidental" value overrides.
+                params.putIfAbsent(parameterName, null);
+            } else {
+                // We flatten the MultivaluedMap values list by emitting the first value only.
+                // We override potential empty parameters that were added to the params map before.
+                params.put(parameterName, parameterValues.get(0));
+            }
+
+        }
+    }
+
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/par/ParEndpointTest.java

@@ -0,0 +1,101 @@
+/*
+ * Copyright 2024 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.testsuite.oauth.par;
+
+import jakarta.ws.rs.core.MultivaluedHashMap;
+import org.junit.Assert;
+import org.junit.Test;
+import org.keycloak.protocol.oidc.par.endpoints.ParEndpoint;
+
+
+import java.util.HashMap;
+import java.util.List;
+
+import static org.keycloak.protocol.oidc.OIDCLoginProtocol.AUTHORIZATION_DETAILS_PARAM;
+
+public class ParEndpointTest {
+
+    @Test
+    public void testFlattenDecodedFormParametersRetainAuthorizationDetails() {
+        var decodedFormParameters = new MultivaluedHashMap<String, String>();
+        String authorizationDetails = "[{\"type\": \"urn:openfinanceuae:account-access-consent:v1.0\",\"foo\":\"bar\"},{\"type\": \"urn:openfinanceuae:account-access-consent:v1.0\",\"gugu\":\"gaga\"}]";
+        decodedFormParameters.put(AUTHORIZATION_DETAILS_PARAM, List.of(authorizationDetails));
+        var params = new HashMap<String, String>();
+
+        ParEndpoint.flattenDecodedFormParametersToParamsMap(decodedFormParameters, params);
+
+        Assert.assertEquals(authorizationDetails, params.get(AUTHORIZATION_DETAILS_PARAM));
+    }
+
+    @Test
+    public void testFlattenDecodedFormParametersMultipleValues() {
+        var decodedFormParameters = new MultivaluedHashMap<String, String>();
+        decodedFormParameters.put("param", List.of("paramValue1", "paramValue2"));
+        var params = new HashMap<String, String>();
+
+        ParEndpoint.flattenDecodedFormParametersToParamsMap(decodedFormParameters, params);
+
+        Assert.assertEquals("paramValue1", params.get("param"));
+    }
+
+    @Test
+    public void testFlattenDecodedFormParametersSingleValue() {
+        var decodedFormParameters = new MultivaluedHashMap<String, String>();
+        decodedFormParameters.put("param", List.of("single"));
+        var params = new HashMap<String, String>();
+
+        ParEndpoint.flattenDecodedFormParametersToParamsMap(decodedFormParameters, params);
+
+        Assert.assertEquals("single", params.get("param"));
+    }
+
+    @Test
+    public void testFlattenDecodedFormParametersNullValue() {
+        var decodedFormParameters = new MultivaluedHashMap<String, String>();
+        decodedFormParameters.add("param", null);
+        var params = new HashMap<String, String>();
+
+        ParEndpoint.flattenDecodedFormParametersToParamsMap(decodedFormParameters, params);
+
+        Assert.assertNull(params.get("param"));
+    }
+
+    @Test
+    public void testFlattenDecodedFormParametersValueWithNull() {
+        var decodedFormParameters = new MultivaluedHashMap<String, String>();
+        decodedFormParameters.add("param", "value");
+        decodedFormParameters.add("param", null);
+        var params = new HashMap<String, String>();
+
+        ParEndpoint.flattenDecodedFormParametersToParamsMap(decodedFormParameters, params);
+
+        Assert.assertEquals("value", params.get("param"));
+    }
+
+    @Test
+    public void testFlattenDecodedFormParametersMultipleDistinctValues() {
+        var decodedFormParameters = new MultivaluedHashMap<String, String>();
+        decodedFormParameters.add("param", "valueAAA");
+        decodedFormParameters.add("param", "valueBBB");
+        var params = new HashMap<String, String>();
+
+        ParEndpoint.flattenDecodedFormParametersToParamsMap(decodedFormParameters, params);
+
+        Assert.assertEquals("valueAAA", params.get("param"));
+    }
+}