Correct handling of --host/--port client options used with --ssl ECFL…

…OW-1985

libs/attribute/CMakeLists.txt

@@ -39,6 +39,7 @@ ecbuild_add_test(
     ../core/test
   LIBS
     ecflow_all
+    test_scaffold
   TEST_DEPENDS
     u_core
     Boost::boost # Boost header-only libraries must be available (namely unit_test_framework)

libs/attribute/test/TestAttrSerialization.cpp

@@ -10,7 +10,6 @@
 
 #include <boost/test/unit_test.hpp>
 
-#include "TestSerialisation.hpp"
 #include "ecflow/attribute/AutoArchiveAttr.hpp"
 #include "ecflow/attribute/AutoCancelAttr.hpp"
 #include "ecflow/attribute/ClockAttr.hpp"
@@ -28,6 +27,7 @@
 #include "ecflow/attribute/VerifyAttr.hpp"
 #include "ecflow/attribute/ZombieAttr.hpp"
 #include "ecflow/core/Calendar.hpp"
+#include "ecflow/test/scaffold/Serialisation.hpp"
 
 using namespace std;
 using namespace ecf;

libs/attribute/test/TestMigration.cpp

@@ -11,7 +11,6 @@
 #include <boost/date_time/posix_time/posix_time_types.hpp>
 #include <boost/test/unit_test.hpp>
 
-#include "TestSerialisation.hpp"
 #include "ecflow/attribute/AutoArchiveAttr.hpp"
 #include "ecflow/attribute/AutoCancelAttr.hpp"
 #include "ecflow/attribute/ClockAttr.hpp"
@@ -31,6 +30,7 @@
 #include "ecflow/core/File.hpp"
 #include "ecflow/core/TimeSlot.hpp"
 #include "ecflow/core/cereal_boost_time.hpp"
+#include "ecflow/test/scaffold/Serialisation.hpp"
 
 using namespace std;
 using namespace ecf;

libs/base/src/ecflow/base/Openssl.cpp

@@ -161,9 +161,15 @@ std::string Openssl::get_password() const {
 }
 
 std::string Openssl::certificates_dir() const {
-    std::string home_path = getenv("HOME");
-    home_path += "/.ecflowrc/ssl/";
-    return home_path;
+    if (auto found = getenv("ECF_SSL_DIR"); found) {
+        // This is used for testing, to avoid using the default location
+        return found;
+    }
+    else {
+        std::string home_path = getenv("HOME");
+        home_path += "/.ecflowrc/ssl/";
+        return home_path;
+    }
 }
 
 std::string Openssl::pem() const {

libs/client/CMakeLists.txt

@@ -56,6 +56,7 @@ set(test_srcs
   test/InvokeServer.hpp
   # Sources
   test/TestClient_main.cpp # test entry point
+  test/TestClientConfigurations.cpp
   test/TestClientEnvironment.cpp
   test/TestClientOptions.cpp
   test/TestClientInterface.cpp
@@ -101,6 +102,7 @@ ecbuild_add_test(
     ../node/test
   LIBS
     ecflow_all
+    test_scaffold
     Boost::boost # Boost header-only libraries must be available (namely unit_test_framework)
     Boost::timer
     $<$<BOOL:${OPENSSL_FOUND}>:OpenSSL::SSL>

libs/client/src/ecflow/client/ClientEnvironment.cpp

@@ -169,12 +169,13 @@ void ClientEnvironment::set_host_port(const std::string& the_host, const std::st
     // When there is only one host:port in host_vec_, calling get_next_host() will always return host_vec_[0]
     host_file_read_ = true;
 
-#ifdef ECF_OPENSSL
-    // Must be done *AFTER* host and port set
-    // Avoid enabling SSL for the GUI, via environment, this must be done explicitly by the GUI
-    if (!gui_)
-        enable_ssl_if_defined();
-#endif
+    // Caution:
+    //
+    //   We don't (re)enable SSL immediatelly after setting host/port, as this might happen multiple times
+    //   during the execution (e.g. when loading environment variables, and later processing command line options).
+    //
+    //   It is up to the user of this class to enable SSL if needed.
+    //
 }
 
 bool ClientEnvironment::checkTaskPathAndPassword(std::string& errorMsg) const {
@@ -311,15 +312,6 @@ void ClientEnvironment::read_environment_variables() {
         host_vec_.clear(); // remove previous setting if any
         host_vec_.emplace_back(host, port);
     }
-
-#ifdef ECF_OPENSSL
-    // Note: This must be placed here for child commands, where we we typically only use environment variables
-    // Must be done last *AFTER* host and port set
-    // Can't use enable_sll(), since that calls host()/port() which use host_vec_, which may be empty
-    // Avoid enabling SSL for the GUI, via environment, this must be done explicitly by the GUI
-    if (!gui_)
-        ssl_.enable_if_defined(host, port);
-#endif
 }
 
 bool ClientEnvironment::parseHostsFile(std::string& errorMsg) {

libs/client/src/ecflow/client/ClientEnvironment.hpp

@@ -93,14 +93,12 @@ class ClientEnvironment final : public AbstractClientEnv {
 #ifdef ECF_OPENSSL
     /// return true if this is a ssl enabled server
     ecf::Openssl& openssl() { return ssl_; }
+    const ecf::Openssl& openssl() const { return ssl_; }
     bool ssl() const { return ssl_.enabled(); }
     void enable_ssl_if_defined() {
         ssl_.enable_if_defined(host(), port());
     }                                                  // IF ECF_SSL=1,search server.crt, ELSE search <host>.<port>.crt
     void enable_ssl() { ssl_.enable(host(), port()); } // search server.crt first, then <host>.<port>.crt
-    bool enable_ssl_no_throw() {
-        return ssl_.enable_no_throw(host(), port());
-    }                                      // search server.crt first, then <host>.<port>.crt
     void disable_ssl() { ssl_.disable(); } // override environment setting for ECF_SSL
 #endif
 

libs/client/src/ecflow/client/ClientInvoker.cpp

@@ -202,6 +202,14 @@ void ClientInvoker::set_connection_attempts(unsigned int attempts) {
         connection_attempts_ = 1;
 }
 
+std::optional<Cmd_ptr> ClientInvoker::get_cmd_from_args(const CommandLine& cl) const {
+    Cmd_ptr cts_cmd;
+    if (get_cmd_from_args(cl, cts_cmd) == 1) {
+        return std::nullopt;
+    }
+    return cts_cmd;
+}
+
 int ClientInvoker::get_cmd_from_args(const CommandLine& cl, Cmd_ptr& cts_cmd) const {
     try {
         // read in program option, and construct the client to server commands from them.
@@ -267,8 +275,9 @@ int ClientInvoker::invoke(const CommandLine& cl) const {
     server_reply_.get_error_msg().clear();
 
     Cmd_ptr cts_cmd;
-    if (get_cmd_from_args(cl, cts_cmd) == 1)
+    if (get_cmd_from_args(cl, cts_cmd) == 1) {
         return 1;
+    }
     if (!cts_cmd)
         return 0; // For --help and --debug, --load defs check_only,  no command is created
 

libs/client/src/ecflow/client/ClientInvoker.hpp

@@ -11,6 +11,8 @@
 #ifndef ecflow_client_ClientInvoker_HPP
 #define ecflow_client_ClientInvoker_HPP
 
+#include <optional>
+
 #include <boost/date_time/posix_time/posix_time_types.hpp>
 
 #include "ecflow/base/Cmd.hpp"
@@ -42,6 +44,8 @@ class ClientInvoker {
     ClientInvoker(const std::string& host, const std::string& port);
     ClientInvoker(const std::string& host, int port);
 
+    const ClientEnvironment& environment() const { return clientEnv_; }
+
     /// for debug allow the current client environment to be printed
     std::string to_string() const { return clientEnv_.toString(); }
 
@@ -69,7 +73,6 @@ class ClientInvoker {
 #ifdef ECF_OPENSSL
     /// Override any ssl read from environment(ECF_SSL) or command line args(-ssl)
     void enable_ssl() { clientEnv_.enable_ssl(); }
-    bool enable_ssl_no_throw() { return clientEnv_.enable_ssl_no_throw(); }
     void disable_ssl() { clientEnv_.disable_ssl(); } // override environment setting for ECF_SSL
 #endif
 
@@ -396,7 +399,13 @@ class ClientInvoker {
                            bool alias = false,
                            bool run   = true); // ecFlowview SUBMIT_FILE
 
+    std::optional<Cmd_ptr> get_cmd_from_args(const CommandLine& cl) const;
+
 private:
+    /**
+     * @return 1 when command is selected; 0 if no command is selected (e.g. --help)
+     * @throws std::runtime_error if the command could not be selected
+     */
     int get_cmd_from_args(const CommandLine& cl, Cmd_ptr& cts_cmd) const;
 
     /// returns 1 on error and 0 on success. The errorMsg can be accessed via errorMsg()

libs/client/src/ecflow/client/ClientOptions.cpp

@@ -178,10 +178,29 @@ Cmd_ptr ClientOptions::parse(const CommandLine& cl, ClientEnvironment* env) cons
     }
 
 #ifdef ECF_OPENSSL
-    if (vm.count("ssl")) {
-        if (env->debug())
-            std::cout << "  ssl set via command line\n";
-        env->enable_ssl();
+    if (auto ecf_ssl = getenv("ECF_SSL"); vm.count("ssl") || ecf_ssl) {
+        if (!vm.count("ssl") && ecf_ssl) {
+            if (env->debug()) {
+                std::cout << "  ssl enabled via environment variable\n";
+            }
+            env->enable_ssl_if_defined();
+        }
+        else if (vm.count("ssl") && !ecf_ssl) {
+            if (env->debug()) {
+                std::cout << "  ssl explicitly enabled via command line\n";
+            }
+            env->enable_ssl();
+        }
+        else {
+            if (env->debug()) {
+                std::cout << "  ssl explicitly enabled via command line, but also enabled via environment variable\n";
+            }
+            env->enable_ssl();
+        }
+
+        if (env->debug()) {
+            std::cout << "  ssl certificate: '" << env->openssl().info() << "' \n";
+        }
     }
 #endif