From c2f7ead468d5e65c57e7ecb49d7fbba4254c46b7 Mon Sep 17 00:00:00 2001 From: Paul Cristian Sarbu Date: Tue, 2 Jul 2024 16:47:13 +0200 Subject: Replace the Auth and Auth::TLS singletons Use a builder pattern for creation and validation, in a manner that allows also other authentication methods to be added in the future besides the current TLS/SSL. The main Auth instances are built early and then passed by not_null const pointers, to avoid passing temporaries, replacing the previous Auth::TLS instances passed by simple nullable const pointers. Where needed, these passed Auth instances are also stored, by const ref. Tests also build Auth instances as needed, either with the default 'no certification' or from the test environment arguments. --- src/other_tools/just_mr/TARGETS | 4 +-- src/other_tools/just_mr/fetch.cpp | 9 +++--- src/other_tools/just_mr/setup.cpp | 9 +++--- src/other_tools/just_mr/setup_utils.cpp | 53 ++++++++++++--------------------- src/other_tools/just_mr/setup_utils.hpp | 5 +++- 5 files changed, 33 insertions(+), 47 deletions(-) (limited to 'src/other_tools') diff --git a/src/other_tools/just_mr/TARGETS b/src/other_tools/just_mr/TARGETS index b8afc631..a2878802 100644 --- a/src/other_tools/just_mr/TARGETS +++ b/src/other_tools/just_mr/TARGETS @@ -82,7 +82,8 @@ , "hdrs": ["setup_utils.hpp"] , "srcs": ["setup_utils.cpp"] , "deps": - [ ["src/buildtool/build_engine/expression", "expression_ptr_interface"] + [ ["src/buildtool/auth", "auth"] + , ["src/buildtool/build_engine/expression", "expression_ptr_interface"] , ["src/buildtool/build_engine/expression", "expression"] , ["src/buildtool/serve_api/remote", "config"] , "cli" @@ -94,7 +95,6 @@ , ["src/buildtool/logging", "log_level"] , ["src/buildtool/logging", "logging"] , "exit_codes" - , ["src/buildtool/auth", "auth"] ] } , "fetch": diff --git a/src/other_tools/just_mr/fetch.cpp b/src/other_tools/just_mr/fetch.cpp index 11ae3a01..07a03f3d 100644 --- a/src/other_tools/just_mr/fetch.cpp +++ b/src/other_tools/just_mr/fetch.cpp @@ -398,14 +398,13 @@ auto MultiRepoFetch(std::shared_ptr const& config, common_args.remote_serve_address); // setup authentication - JustMR::Utils::SetupAuthConfig(auth_args); - std::optional auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); + auto auth_config = JustMR::Utils::CreateAuthConfig(auth_args); + if (not auth_config) { + return kExitConfigError; } ApiBundle const apis{/*repo_config=*/nullptr, - auth ? &*auth : nullptr, + &*auth_config, RemoteExecutionConfig::RemoteAddress()}; bool const has_remote_api = diff --git a/src/other_tools/just_mr/setup.cpp b/src/other_tools/just_mr/setup.cpp index 70c74eb0..6866bc26 100644 --- a/src/other_tools/just_mr/setup.cpp +++ b/src/other_tools/just_mr/setup.cpp @@ -117,14 +117,13 @@ auto MultiRepoSetup(std::shared_ptr const& config, common_args.remote_serve_address); // setup authentication - JustMR::Utils::SetupAuthConfig(auth_args); - std::optional auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); + auto auth_config = JustMR::Utils::CreateAuthConfig(auth_args); + if (not auth_config) { + return std::nullopt; } ApiBundle const apis{/*repo_config=*/nullptr, - auth ? &*auth : nullptr, + &*auth_config, RemoteExecutionConfig::RemoteAddress()}; bool const has_remote_api = diff --git a/src/other_tools/just_mr/setup_utils.cpp b/src/other_tools/just_mr/setup_utils.cpp index 7ed61897..577fb1bf 100644 --- a/src/other_tools/just_mr/setup_utils.cpp +++ b/src/other_tools/just_mr/setup_utils.cpp @@ -19,7 +19,6 @@ #include #include "nlohmann/json.hpp" -#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/build_engine/expression/expression.hpp" #include "src/buildtool/file_system/file_system_manager.hpp" #include "src/buildtool/logging/log_level.hpp" @@ -193,42 +192,28 @@ auto ReadConfiguration( } } -void SetupAuthConfig(MultiRepoRemoteAuthArguments const& authargs) noexcept { - bool use_tls{false}; - if (authargs.tls_ca_cert) { - use_tls = true; - if (not Auth::TLS::Instance().SetCACertificate(*authargs.tls_ca_cert)) { - Logger::Log(LogLevel::Error, - "Could not read '{}' certificate.", - authargs.tls_ca_cert->string()); - std::exit(kExitConfigError); - } - } - if (authargs.tls_client_cert) { - use_tls = true; - if (not Auth::TLS::Instance().SetClientCertificate( - *authargs.tls_client_cert)) { - Logger::Log(LogLevel::Error, - "Could not read '{}' certificate.", - authargs.tls_client_cert->string()); - std::exit(kExitConfigError); - } - } - if (authargs.tls_client_key) { - use_tls = true; - if (not Auth::TLS::Instance().SetClientKey(*authargs.tls_client_key)) { - Logger::Log(LogLevel::Error, - "Could not read '{}' key.", - authargs.tls_client_key->string()); - std::exit(kExitConfigError); - } - } +auto CreateAuthConfig(MultiRepoRemoteAuthArguments const& authargs) noexcept + -> std::optional { - if (use_tls) { - if (not Auth::TLS::Instance().Validate()) { - std::exit(kExitConfigError); + Auth::TLS::Builder tls_builder; + tls_builder.SetCACertificate(authargs.tls_ca_cert) + .SetClientCertificate(authargs.tls_client_cert) + .SetClientKey(authargs.tls_client_key); + + // create auth config (including validation) + auto result = tls_builder.Build(); + if (result) { + if (*result) { + // correctly configured TLS/SSL certification + return *std::move(*result); } + Logger::Log(LogLevel::Error, result->error()); + return std::nullopt; } + + // no TLS/SSL configuration was given, and we currently support no other + // certification method, so return an empty config (no certification) + return Auth{}; } void SetupRemoteConfig( diff --git a/src/other_tools/just_mr/setup_utils.hpp b/src/other_tools/just_mr/setup_utils.hpp index 2ae08d41..3043855c 100644 --- a/src/other_tools/just_mr/setup_utils.hpp +++ b/src/other_tools/just_mr/setup_utils.hpp @@ -21,6 +21,7 @@ #include #include +#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/build_engine/expression/configuration.hpp" #include "src/buildtool/build_engine/expression/expression_ptr.hpp" #include "src/buildtool/serve_api/remote/config.hpp" @@ -60,7 +61,9 @@ void DefaultReachableRepositories( std::optional const& absent_file_opt) noexcept -> std::shared_ptr; -void SetupAuthConfig(MultiRepoRemoteAuthArguments const& authargs) noexcept; +[[nodiscard]] auto CreateAuthConfig( + MultiRepoRemoteAuthArguments const& authargs) noexcept + -> std::optional; void SetupRemoteConfig( std::optional const& remote_exec_addr, -- cgit v1.2.3