FISH-5745 load extra params from mp config, allow multiple keys

openid/src/main/java/fish/payara/security/openid/controller/AuthenticationController.java

@@ -126,7 +126,11 @@ public AuthenticationStatus authenticateUser(
             authRequest.queryParam(OpenIdConstant.PROMPT, configuration.getPrompt());
         }
 
-        configuration.getExtraParameters().forEach(authRequest::queryParam);
+        configuration.getExtraParameters().forEach(
+                (key, values) -> values.stream().forEach(
+                        value -> authRequest.queryParam(key, value)
+                )
+        );
 
         String authUrl = authRequest.toString();
         LOGGER.log(FINEST, "Redirecting for authentication to {0}", authUrl);

openid/src/main/java/fish/payara/security/openid/controller/ConfigurationController.java

@@ -50,9 +50,7 @@
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 import java.util.stream.Stream;
 
@@ -68,6 +66,11 @@
 import fish.payara.security.openid.OpenIdAuthenticationException;
 import fish.payara.security.openid.OpenIdUtil;
 import fish.payara.security.openid.api.OpenIdConstant;
+import java.net.URLDecoder;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+import java.util.LinkedHashMap;
+import java.util.Map;
 import org.eclipse.microprofile.config.Config;
 import org.eclipse.microprofile.config.ConfigProvider;
 
@@ -189,13 +192,7 @@ public OpenIdConfiguration buildConfig(OpenIdAuthenticationDefinition definition
                 .collect(joining(SPACE_SEPARATOR));
         prompt = OpenIdUtil.getConfiguredValue(String.class, prompt, provider, OpenIdAuthenticationDefinition.OPENID_MP_PROMPT);
 
-        Map<String, String> extraParameters = new HashMap<>();
-        for (String extraParameter : definition.extraParameters()) {
-            String[] parts = extraParameter.split("=");
-            String key = parts[0];
-            String value = parts[1];
-            extraParameters.put(key, value);
-        }
+        String extraParametersFromAnnotation = createUrlQuery("extraParameters", definition.extraParameters());
 
         boolean useNonce = OpenIdUtil.getConfiguredValue(Boolean.class, definition.useNonce(), provider, OpenIdAuthenticationDefinition.OPENID_MP_USE_NONCE);
         boolean useSession = OpenIdUtil.getConfiguredValue(Boolean.class, definition.useSession(), provider, OpenIdAuthenticationDefinition.OPENID_MP_USE_SESSION);
@@ -218,6 +215,8 @@ public OpenIdConfiguration buildConfig(OpenIdAuthenticationDefinition definition
         boolean tokenAutoRefresh = OpenIdUtil.getConfiguredValue(Boolean.class, definition.tokenAutoRefresh(), provider, OpenIdAuthenticationDefinition.OPENID_MP_TOKEN_AUTO_REFRESH);
         int tokenMinValidity = OpenIdUtil.getConfiguredValue(Integer.class, definition.tokenMinValidity(), provider, OpenIdAuthenticationDefinition.OPENID_MP_TOKEN_MIN_VALIDITY);
         boolean userClaimsFromIDToken = OpenIdUtil.getConfiguredValue(Boolean.class, definition.userClaimsFromIDToken(), provider, OpenIdAuthenticationDefinition.OPENID_MP_USER_CLAIMS_FROM_ID_TOKEN);
+        String extraParamsRaw = OpenIdUtil.getConfiguredValue(String.class, extraParametersFromAnnotation, provider, OpenIdAuthenticationDefinition.OPENID_MP_EXTRA_PARAMS_RAW);
+        Map<String, List<String>> extraParameters = parseMultiMapFromUrlQuery(extraParamsRaw);
 
         fish.payara.security.openid.domain.OpenIdProviderMetadata openIdProviderMetadata = new fish.payara.security.openid.domain.OpenIdProviderMetadata(
                 providerDocument,
@@ -365,6 +364,47 @@ private List<String> validateClientConfiguration(OpenIdConfiguration configurati
         return errorMessages;
     }
 
+    /**
+     * Create Url query from pairs of parameters.
+     */
+    public static String createUrlQuery(String paramsName, String[] parameters) {
+        StringBuilder extraParametersFromAnnotationBuf = new StringBuilder();
+        String extraParamDelim = "";
+        for (String extraParameter : parameters) {
+            String[] parts = extraParameter.split("=");
+            if (parts.length == 0 || parts[0].length() == 0) {
+                throw new OpenIdAuthenticationException(paramsName + " contain parameter without key: '" + extraParameter + "', expected format: key=value");
+            }
+            String key = parts[0];
+            String value = parts.length > 1 ? parts[1] : null;
+            extraParametersFromAnnotationBuf.append(extraParamDelim)
+                    .append(URLEncoder.encode(key, StandardCharsets.UTF_8));
+            if (value != null) {
+                extraParametersFromAnnotationBuf
+                        .append("=")
+                        .append(URLEncoder.encode(value, StandardCharsets.UTF_8));
+            }
+            extraParamDelim = "&";
+        }
+        return extraParametersFromAnnotationBuf.toString();
+    }
+
+    /**
+     * Parse Url query format to multimap.
+     */
+    public static Map<String, List<String>> parseMultiMapFromUrlQuery(String query) {
+        Map<String, List<String>> multiMap = new LinkedHashMap<>();
+        String[] pairs = query.split("&");
+        for (String pair : pairs) {
+            String[] keyValue = pair.split("=");
+            String key = keyValue[0];
+            String value = keyValue.length > 1 ? URLDecoder.decode(keyValue[1], StandardCharsets.UTF_8) : null;
+            List<String> values = multiMap.computeIfAbsent(key, k -> new ArrayList<>());
+            values.add(value);
+        }
+        return multiMap;
+    }
+
     static class LastBuiltConfig {
         private final OpenIdAuthenticationDefinition definition;
         private final OpenIdConfiguration configuration;

openid/src/main/java/fish/payara/security/openid/domain/OpenIdConfiguration.java

@@ -42,6 +42,7 @@
 import javax.servlet.http.HttpServletRequest;
 
 import fish.payara.security.openid.controller.JWTValidator;
+import java.util.List;
 
 /**
  * OpenId Connect client configuration
@@ -56,7 +57,7 @@ public class OpenIdConfiguration {
     private String scopes;
     private String responseType;
     private String responseMode;
-    private Map<String, String> extraParameters;
+    private Map<String, List<String>> extraParameters;
     private String prompt;
     private String display;
     private boolean useNonce;
@@ -137,11 +138,11 @@ public OpenIdConfiguration setResponseMode(String responseMode) {
         return this;
     }
 
-    public Map<String, String> getExtraParameters() {
+    public Map<String, List<String>> getExtraParameters() {
         return extraParameters;
     }
 
-    public OpenIdConfiguration setExtraParameters(Map<String, String> extraParameters) {
+    public OpenIdConfiguration setExtraParameters(Map<String, List<String>> extraParameters) {
         this.extraParameters = extraParameters;
         return this;
     }

openid/src/test/java/fish/payara/security/openid/domain/ConfigurationControllerTest.java

@@ -0,0 +1,151 @@
+/*
+ * Copyright (c) 2021 Payara Foundation and/or its affiliates. All rights reserved.
+ *
+ *  The contents of this file are subject to the terms of either the GNU
+ *  General Public License Version 2 only ("GPL") or the Common Development
+ *  and Distribution License("CDDL") (collectively, the "License").  You
+ *  may not use this file except in compliance with the License.  You can
+ *  obtain a copy of the License at
+ *  https://github.com/payara/Payara/blob/master/LICENSE.txt
+ *  See the License for the specific
+ *  language governing permissions and limitations under the License.
+ *
+ *  When distributing the software, include this License Header Notice in each
+ *  file and include the License file at glassfish/legal/LICENSE.txt.
+ *
+ *  GPL Classpath Exception:
+ *  The Payara Foundation designates this particular file as subject to the "Classpath"
+ *  exception as provided by the Payara Foundation in the GPL Version 2 section of the License
+ *  file that accompanied this code.
+ *
+ *  Modifications:
+ *  If applicable, add the following below the License Header, with the fields
+ *  enclosed by brackets [] replaced by your own identifying information:
+ *  "Portions Copyright [year] [name of copyright owner]"
+ *
+ *  Contributor(s):
+ *  If you wish your version of this file to be governed by only the CDDL or
+ *  only the GPL Version 2, indicate your decision by adding "[Contributor]
+ *  elects to include this software in this distribution under the [CDDL or GPL
+ *  Version 2] license."  If you don't indicate a single choice of license, a
+ *  recipient has the option to distribute your version of this file under
+ *  either the CDDL, the GPL Version 2 or to extend the choice of license to
+ *  its licensees as provided above.  However, if you add GPL Version 2 code
+ *  and therefore, elected the GPL Version 2 license, then the option applies
+ *  only if the new code is made subject to such option by the copyright
+ *  holder.
+ */
+package fish.payara.security.openid.domain;
+
+import fish.payara.security.openid.OpenIdAuthenticationException;
+import fish.payara.security.openid.controller.ConfigurationController;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.junit.jupiter.api.Assertions;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import org.junit.jupiter.api.Test;
+import org.junit.platform.runner.JUnitPlatform;
+import org.junit.runner.RunWith;
+
+/**
+ * Test ConfigurationController.
+ *
+ * @author Petr Aubrecht <petr@aubrecht.net>
+ */
+@RunWith(JUnitPlatform.class)
+public class ConfigurationControllerTest {
+
+    @Test
+    public void createUrlNoParameter() {
+        assertEquals("",
+                ConfigurationController.createUrlQuery("extraParameters", new String[]{}));
+    }
+
+    @Test
+    public void createUrlOneParameter() {
+        assertEquals("a=b",
+                ConfigurationController.createUrlQuery("extraParameters", new String[]{"a=b"}));
+    }
+
+    @Test
+    public void createUrlSimpleParameters() {
+        assertEquals("a=b&c=d",
+                ConfigurationController.createUrlQuery("extraParameters", new String[]{"a=b", "c=d"}));
+    }
+
+    @Test
+    public void createUrlNoValueParameters() {
+        assertEquals("a&c=d&e",
+                ConfigurationController.createUrlQuery("extraParameters", new String[]{"a", "c=d", "e"}));
+    }
+
+    @Test
+    public void createUrlWithSpacesParameters() {
+        assertEquals("a=b+b&c=++d++&e",
+                ConfigurationController.createUrlQuery("extraParameters", new String[]{"a=b b", "c=  d  ", "e="}));
+    }
+
+    @Test
+    public void createUrlWithoutKeyNameParameters() {
+        // these cases are cought by OpenIdExtension anyway, just defensive test:
+        Assertions.assertThrows(OpenIdAuthenticationException.class,
+                () -> ConfigurationController.createUrlQuery("extraParameters", new String[]{""}));
+        Assertions.assertThrows(OpenIdAuthenticationException.class,
+                () -> ConfigurationController.createUrlQuery("extraParameters", new String[]{"="}));
+        Assertions.assertThrows(OpenIdAuthenticationException.class,
+                () -> ConfigurationController.createUrlQuery("extraParameters", new String[]{"=a"}));
+    }
+
+    @Test
+    public void createUrlWithBlankKeyNameParameters() {
+        assertEquals("+=+",
+                ConfigurationController.createUrlQuery("extraParameters", new String[]{" = "}));
+    }
+
+    @Test
+    public void parseMultiMapFromUrlQuery() {
+        Map<String, List<String>> result = new HashMap<>();
+        result.put("a", Arrays.asList("b"));
+        assertEquals(result, ConfigurationController.parseMultiMapFromUrlQuery("a=b"));
+    }
+
+    @Test
+    public void parseMultiMapFromUrlQueryNoValue() {
+        Map<String, List<String>> result = new HashMap<>();
+        result.put("a", Arrays.asList((String) null));
+        assertEquals(result, ConfigurationController.parseMultiMapFromUrlQuery("a="));
+    }
+
+    @Test
+    public void parseMultiMapFromUrlQueryNoValueWithoutEquals() {
+        Map<String, List<String>> result = new HashMap<>();
+        result.put("a", Arrays.asList((String) null));
+        assertEquals(result, ConfigurationController.parseMultiMapFromUrlQuery("a"));
+    }
+
+    @Test
+    public void parseMultiMapFromUrlQueryMultipleValues() {
+        Map<String, List<String>> result = new HashMap<>();
+        result.put("a", Arrays.asList("b", "c"));
+        assertEquals(result, ConfigurationController.parseMultiMapFromUrlQuery("a=b&a=c"));
+    }
+
+    @Test
+    public void parseMultiMapFromUrlQueryMultipleKeysMultipleValues() {
+        Map<String, List<String>> result = new HashMap<>();
+        result.put("a", Arrays.asList("b", "c"));
+        result.put("x", Arrays.asList("y", "z"));
+        assertEquals(result, ConfigurationController.parseMultiMapFromUrlQuery("a=b&a=c&x=y&x=z"));
+    }
+
+    @Test
+    public void parseMultiMapFromUrlQueryMultipleKeysWithSpaces() {
+        Map<String, List<String>> result = new HashMap<>();
+        result.put("a", Arrays.asList("b b"));
+        result.put("c", Arrays.asList("  d  "));
+        result.put("e", Arrays.asList((String) null));
+        assertEquals(result, ConfigurationController.parseMultiMapFromUrlQuery("a=b+b&c=++d++&e"));
+    }
+}

security-connectors-api/src/main/java/fish/payara/security/annotations/OpenIdAuthenticationDefinition.java

@@ -192,6 +192,9 @@
      * These must be in the form of {@code "key=value"} i.e.
      * <code> extraParameters={"key1=value", "key2=value2"} </code>
      *
+     * To set this using Microprofile Config use {@code payara.security.openid.extraParams.raw}, in URL query format:
+     * {@code key=value&key2=value+with+spaces}. The keys may repeat.
+     *
      * @return
      */
     String[] extraParameters() default {};
@@ -372,4 +375,10 @@
      * and get the user information from ID Token is <code>{@value}</code>
      */
     String OPENID_MP_USER_CLAIMS_FROM_ID_TOKEN = "payara.security.openid.userClaimsFromIDToken";
+
+    /**
+     * The Microprofile Config key for extraParams is <code>{@value}</code>. Use URL query format to store key/value
+     * pairs: {@code key=value&key2=value+with+spaces}. The keys may repeat.
+     */
+    String OPENID_MP_EXTRA_PARAMS_RAW = "payara.security.openid.extraParams.raw";
 }