[KEYCLOAK-14255] - More improvements to CLI

quarkus/runtime/src/main/java/org/keycloak/cli/MainCommand.java

@@ -21,6 +21,7 @@ import org.keycloak.configuration.KeycloakConfigSourceProvider;
 
 import io.quarkus.bootstrap.runner.QuarkusEntryPoint;
 import io.quarkus.runtime.Quarkus;
+import org.keycloak.util.Environment;
 import picocli.CommandLine;
 import picocli.CommandLine.Command;
 import picocli.CommandLine.Model.CommandSpec;
@@ -67,15 +68,30 @@ public class MainCommand {
             usageHelpAutoWidth = true,
             optionListHeading = "%nOptions%n",
             parameterListHeading = "Available Commands%n")
-    public void reAugment() {
+    public void reAugment(@Option(names = "--verbose", description = "Print out more details when running this command.", required = false) Boolean debug) {
         System.setProperty("quarkus.launch.rebuild", "true");
         println("Updating the configuration and installing your custom providers, if any. Please wait.");
         try {
             QuarkusEntryPoint.main();
+            println("Server configuration updated and persisted. Run the following command to review the configuration:\n");
+            println("\t" + Environment.getCommand() + " show-config\n");
         } catch (Throwable throwable) {
-            error("Failed to update server configuration.");
+            String message = throwable.getMessage();
-        } finally {
+            Throwable cause = throwable.getCause();
-            System.exit(CommandLine.ExitCode.OK);
+
+            if (cause != null) {
+                message = cause.getMessage();                
+            }
+            
+            error("Failed to update server configuration: " + message);
+            
+            if (debug == null) {
+                errorAndExit("For more details run the same command passing the '--verbose' option.");
+            } else {
+                error("Details:");
+                throwable.printStackTrace();
+                System.exit(spec.exitCodeOnExecutionException());
+            }
         }
     }
 
@@ -110,7 +126,7 @@ public class MainCommand {
             System.setProperty("keycloak.migration.provider", "singleFile");
             System.setProperty("keycloak.migration.file", toFile);
         } else {
-            error("Must specify either --dir or --file options.");
+            errorAndExit("Must specify either --dir or --file options.");
         }
 
         System.setProperty("keycloak.migration.usersExportStrategy", users.toUpperCase());
@@ -143,7 +159,7 @@ public class MainCommand {
             System.setProperty("keycloak.migration.provider", "singleFile");
             System.setProperty("keycloak.migration.file", toFile);
         } else {
-            error("Must specify either --dir or --file options.");
+            errorAndExit("Must specify either --dir or --file options.");
         }
 
         if (realm != null) {
@@ -193,8 +209,12 @@ public class MainCommand {
         spec.commandLine().getOut().println(message);
     }
 
-    private void error(String message) {
+    private void errorAndExit(String message) {
-        spec.commandLine().getErr().println(message);
+        error(message);
         System.exit(CommandLine.ExitCode.SOFTWARE);
     }
+
+    private void error(String message) {
+        spec.commandLine().getErr().println(message);
+    }
 }

quarkus/runtime/src/main/java/org/keycloak/cli/ShowConfigCommand.java

@@ -18,6 +18,7 @@
 package org.keycloak.cli;
 
 import static java.lang.Boolean.parseBoolean;
+import static org.keycloak.configuration.PropertyMappers.formatValue;
 import static org.keycloak.util.Environment.getBuiltTimeProperty;
 import static org.keycloak.util.Environment.getConfig;
 
@@ -29,7 +30,6 @@ import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
 
 import org.keycloak.configuration.MicroProfileConfigProvider;
-import org.keycloak.quarkus.KeycloakRecorder;
 import org.keycloak.util.Environment;
 
 import io.smallrye.config.ConfigValue;
@@ -112,7 +112,7 @@ public final class ShowConfigCommand {
         String value = getBuiltTimeProperty(property).orElse(null);
 
         if (value != null && !"".equals(value.trim())) {
-            System.out.printf("\t%s =  %s (build-time)%n", property, value);
+            System.out.printf("\t%s =  %s (persisted)%n", property, formatValue(property, value));
             return;
         }
 
@@ -122,7 +122,7 @@ public final class ShowConfigCommand {
             return;
         }
 
-        System.out.printf("\t%s =  %s (%s)%n", property, configValue.getValue(), configValue.getConfigSourceName());
+        System.out.printf("\t%s =  %s (%s)%n", property, formatValue(property, configValue.getValue()), configValue.getConfigSourceName());
     }
 
     private static String groupProperties(String property) {

quarkus/runtime/src/main/java/org/keycloak/configuration/Messages.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2020 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.configuration;
+
+import java.util.List;
+
+public final class Messages {
+
+    static IllegalArgumentException invalidDatabaseVendor(String db, String... availableOptions) {
+        return new IllegalArgumentException("Invalid database vendor [" + db + "]. Possible values are: " + String.join(", ", availableOptions) + ".");
+    }
+
+    static IllegalArgumentException invalidProxyMode(String mode) {
+        return new IllegalArgumentException("Invalid value [" + mode + "] for configuration property [proxy].");
+    }
+
+    static IllegalStateException httpsConfigurationNotSet() {
+        return new IllegalStateException("Key material not provided to setup HTTPS. Please configure your keys/certificates or enable HTTP.");
+    }
+}

quarkus/runtime/src/main/java/org/keycloak/configuration/PropertyMapper.java

@@ -46,12 +46,16 @@ public class PropertyMapper {
         return MAPPERS.computeIfAbsent(toProperty, s -> new PropertyMapper(fromProperty, s, null, transformer, null, description));
     }
 
+    static PropertyMapper create(String fromProperty, String toProperty, String description, boolean mask) {
+        return MAPPERS.computeIfAbsent(toProperty, s -> new PropertyMapper(fromProperty, s, null, null, null, false, description, mask));
+    }
+
     static PropertyMapper create(String fromProperty, String mapFrom, String toProperty, BiFunction<String, ConfigSourceInterceptorContext, String> transformer, String description) {
         return MAPPERS.computeIfAbsent(toProperty, s -> new PropertyMapper(fromProperty, s, null, transformer, mapFrom, description));
     }
 
     static PropertyMapper forBuildTimeProperty(String fromProperty, String toProperty, BiFunction<String, ConfigSourceInterceptorContext, String> transformer, String description) {
-        return MAPPERS.computeIfAbsent(toProperty, s -> new PropertyMapper(fromProperty, s, null, transformer, null, true, description));
+        return MAPPERS.computeIfAbsent(toProperty, s -> new PropertyMapper(fromProperty, s, null, transformer, null, true, description, false));
     }
 
     static Map<String, PropertyMapper> MAPPERS = new HashMap<>();
@@ -81,16 +85,21 @@ public class PropertyMapper {
     private final String mapFrom;
     private final boolean buildTime;
     private String description;
+    private boolean mask;
 
     PropertyMapper(String from, String to, String defaultValue, BiFunction<String, ConfigSourceInterceptorContext, String> mapper, String description) {
         this(from, to, defaultValue, mapper, null, description);
     }
 
-    PropertyMapper(String from, String to, String defaultValue, BiFunction<String, ConfigSourceInterceptorContext, String> mapper, String mapFrom, String description) {
+    PropertyMapper(String from, String to, String defaultValue, BiFunction<String, ConfigSourceInterceptorContext, String> mapper, String description, boolean mask) {
-        this(from, to, defaultValue, mapper, mapFrom, false, description);
+        this(from, to, defaultValue, mapper, null, false, description, mask);
     }
 
-    PropertyMapper(String from, String to, String defaultValue, BiFunction<String, ConfigSourceInterceptorContext, String> mapper, String mapFrom, boolean buildTime, String description) {
+    PropertyMapper(String from, String to, String defaultValue, BiFunction<String, ConfigSourceInterceptorContext, String> mapper, String mapFrom, String description) {
+        this(from, to, defaultValue, mapper, mapFrom, false, description, false);
+    }
+    
+    PropertyMapper(String from, String to, String defaultValue, BiFunction<String, ConfigSourceInterceptorContext, String> mapper, String mapFrom, boolean buildTime, String description, boolean mask) {
         this.from = MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX + from;
         this.to = to;
         this.defaultValue = defaultValue;
@@ -102,6 +111,7 @@ public class PropertyMapper {
         this.mapFrom = mapFrom;
         this.buildTime = buildTime;
         this.description = description;
+        this.mask = mask;
     }
 
     ConfigValue getOrDefault(String name, ConfigSourceInterceptorContext context, ConfigValue current) {
@@ -200,4 +210,12 @@ public class PropertyMapper {
     public String getDescription() {
         return description;
     }
+
+    public boolean isMask() {
+        return mask;
+    }
+
+    public String getTo() {
+        return to;
+    }
 }

quarkus/runtime/src/main/java/org/keycloak/configuration/PropertyMappers.java

@@ -16,11 +16,14 @@
  */
 package org.keycloak.configuration;
 
+import static org.keycloak.configuration.Messages.invalidDatabaseVendor;
+import static org.keycloak.configuration.PropertyMapper.MAPPERS;
 import static org.keycloak.configuration.PropertyMapper.create;
 import static org.keycloak.configuration.PropertyMapper.createWithDefault;
 import static org.keycloak.configuration.PropertyMapper.forBuildTimeProperty;
 
 import java.util.List;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import io.quarkus.runtime.configuration.ProfileManager;
@@ -50,6 +53,18 @@ public final class PropertyMappers {
                 enabled = true;
             }
             
+            if (!enabled) {
+                ConfigValue proceed = context.proceed("kc.https.certificate.file");
+                
+                if (proceed == null || proceed.getValue() == null) {
+                    proceed = context.proceed("kc.https.certificate.key-store-file");
+                }
+                
+                if (proceed == null || proceed.getValue() == null) {
+                    throw Messages.httpsConfigurationNotSet();
+                }
+            }
+            
             return enabled ? "enabled" : "disabled";
         }, "Enables the HTTP listener.");
         createWithDefault("http.port", "quarkus.http.port", String.valueOf(8080), "The HTTP port.");
@@ -59,10 +74,10 @@ public final class PropertyMappers {
         create("https.protocols", "quarkus.http.ssl.protocols", "The list of protocols to explicitly enable.");
         create("https.certificate.file", "quarkus.http.ssl.certificate.file", "The file path to a server certificate or certificate chain in PEM format.");
         create("https.certificate.key-store-file", "quarkus.http.ssl.certificate.key-store-file", "An optional key store which holds the certificate information instead of specifying separate files.");
-        create("https.certificate.key-store-password", "quarkus.http.ssl.certificate.key-store-password", "A parameter to specify the password of the key store file. If not given, the default (\"password\") is used.");
+        create("https.certificate.key-store-password", "quarkus.http.ssl.certificate.key-store-password", "A parameter to specify the password of the key store file. If not given, the default (\"password\") is used.", true);
         create("https.certificate.key-store-file-type", "quarkus.http.ssl.certificate.key-store-file-type", "An optional parameter to specify type of the key store file. If not given, the type is automatically detected based on the file name.");
         create("https.certificate.trust-store-file", "quarkus.http.ssl.certificate.trust-store-file", "An optional trust store which holds the certificate information of the certificates to trust.");
-        create("https.certificate.trust-store-password", "quarkus.http.ssl.certificate.trust-store-password", "A parameter to specify the password of the trust store file.");
+        create("https.certificate.trust-store-password", "quarkus.http.ssl.certificate.trust-store-password", "A parameter to specify the password of the trust store file.", true);
         create("https.certificate.trust-store-file-type", "quarkus.http.ssl.certificate.trust-store-file-type", "An optional parameter to specify type of the trust store file. If not given, the type is automatically detected based on the file name.");
     }
 
@@ -76,7 +91,7 @@ public final class PropertyMappers {
                 case "passthrough":
                     return "true";
             }
-            throw new RuntimeException("Invalid value [" + mode + "] for configuration property [proxy]");
+            throw Messages.invalidProxyMode(mode);
         }, "The proxy mode if the server is behind a reverse proxy. Possible values are: none, edge, reencrypt, and passthrough.");
     }
 
@@ -90,10 +105,11 @@ public final class PropertyMappers {
                     return "org.hibernate.dialect.MariaDBDialect";
                 case "postgres-95":
                     return "io.quarkus.hibernate.orm.runtime.dialect.QuarkusPostgreSQL95Dialect";
+                case "postgres": // shorthand for the recommended postgres version
                 case "postgres-10":
                     return "io.quarkus.hibernate.orm.runtime.dialect.QuarkusPostgreSQL10Dialect";
             }
-            return null;
+            throw invalidDatabaseVendor(db, "h2-file", "h2-mem", "mariadb", "postgres", "postgres-95", "postgres-10");
         }, "The database vendor. Possible values are: h2-mem, h2-file, mariadb, postgres95, postgres10.");
         create("db", "quarkus.datasource.driver", (db, context) -> {
             switch (db.toLowerCase()) {
@@ -112,19 +128,19 @@ public final class PropertyMappers {
         create("db.url", "db", "quarkus.datasource.url", (db, context) -> {
             switch (db.toLowerCase()) {
                 case "h2-file":
-                    return "jdbc:h2:file:${kc.home.dir:${kc.db.url.path:~}}/data/keycloakdb${kc.db.url.properties:;;AUTO_SERVER=TRUE}";
+                    return "jdbc:h2:file:${kc.home.dir:${kc.db.url.path:~}}/${kc.data.dir:data}/keycloakdb${kc.db.url.properties:;;AUTO_SERVER=TRUE}";
                 case "h2-mem":
                     return "jdbc:h2:mem:keycloakdb${kc.db.url.properties:}";
                 case "mariadb":
                     return "jdbc:mariadb://${kc.db.url.host:localhost}/${kc.db.url.database:keycloak}${kc.db.url.properties:}";
                 case "postgres-95":
                 case "postgres-10":
-                    return "jdbc:postgresql://${kc.db.url.host:localhost}/${kc.db.url.database}${kc.db.url.properties:}";
+                    return "jdbc:postgresql://${kc.db.url.host:localhost}/${kc.db.url.database:keycloak}${kc.db.url.properties:}";
             }
             return null;
-        }, "The database JDBC URL. If not provided a default URL is set based on the selected database vendor.");
+        }, "The database JDBC URL. If not provided a default URL is set based on the selected database vendor. For instance, if using 'postgres-10', the JDBC URL would be 'jdbc:postgresql://localhost/keycloak'. The host, database and properties can be overridden by setting the following system properties, respectively: -Dkc.db.url.host, -Dkc.db.url.database, -Dkc.db.url.properties.");
         create("db.username", "quarkus.datasource.username", "The database username.");
-        create("db.password", "quarkus.datasource.password", "The database password");
+        create("db.password", "quarkus.datasource.password", "The database password", true);
         create("db.schema", "quarkus.datasource.schema", "The database schema.");
         create("db.pool.initial-size", "quarkus.datasource.jdbc.initial-size", "The initial size of the connection pool.");
         create("db.pool.min-size", "quarkus.datasource.jdbc.min-size", "The minimal size of the connection pool.");
@@ -181,4 +197,23 @@ public final class PropertyMappers {
     public static String canonicalFormat(String name) {
         return name.replaceAll("-", "\\.");
     }
+
+    public static String formatValue(String property, String value) {
+        PropertyMapper mapper = PropertyMappers.getMapper(property);
+
+        if (mapper != null && mapper.isMask()) {
+            return "*******";
+        }
+
+        return value;
+    }
+    
+    public static PropertyMapper getMapper(String property) {
+        return MAPPERS.values().stream().filter(new Predicate<PropertyMapper>() {
+            @Override
+            public boolean test(PropertyMapper propertyMapper) {
+                return property.equals(propertyMapper.getFrom()) || property.equals(propertyMapper.getTo());
+            }
+        }).findFirst().orElse(null);
+    }
 }

quarkus/runtime/src/test/java/org/keycloak/provider/quarkus/ConfigurationTest.java

@@ -154,7 +154,6 @@ public class ConfigurationTest {
     public void testCommandLineArguments() {
         System.setProperty("kc.config.args", "--spi-hostname-default-frontend-url=http://fromargs.com,--no-ssl");
         assertEquals("http://fromargs.com", initConfig("hostname", "default").get("frontendUrl"));
-        assertEquals("true", ConfigProvider.getConfig().getValue("kc.no-ssl", String.class));
     }
     
     @Test
@@ -202,12 +201,15 @@ public class ConfigurationTest {
 
     @Test
     public void testDatabaseProperties() {
-        System.setProperty("kc.config.args", "--db=h2-file,--db-url-path=test,--db-url-properties=;;test=test;test1=test1");
+        System.setProperty("kc.db.url.properties", ";;test=test;test1=test1");
+        System.setProperty("kc.db.url.path", "test-dir");
+        System.setProperty("kc.config.args", "--db=h2-file");
         SmallRyeConfig config = createConfig();
         assertEquals(QuarkusH2Dialect.class.getName(), config.getConfigValue("quarkus.hibernate-orm.dialect").getValue());
-        assertEquals("jdbc:h2:file:test/data/keycloakdb;;test=test;test1=test1", config.getConfigValue("quarkus.datasource.url").getValue());
+        assertEquals("jdbc:h2:file:test-dir/data/keycloakdb;;test=test;test1=test1", config.getConfigValue("quarkus.datasource.url").getValue());
 
-        System.setProperty("kc.config.args", "--db=mariadb,--db-url-path=test,--db-url-properties=?test=test&test1=test1");
+        System.setProperty("kc.db.url.properties", "?test=test&test1=test1");
+        System.setProperty("kc.config.args", "--db=mariadb");
         config = createConfig();
         assertEquals("jdbc:mariadb://localhost/keycloak?test=test&test1=test1", config.getConfigValue("quarkus.datasource.url").getValue());
     }