From db60f5847bc89e96fbee5eb504726c11382973b8 Mon Sep 17 00:00:00 2001 From: Nagaraju Goruganti Date: Thu, 8 Nov 2018 03:14:48 -0600 Subject: ldap-config: remove Bindpassword and secureLDAP property from the interface This is a reaction to below given phosphor-dbus-interfaces changes https://gerrit.openbmc-project.xyz/#/c/openbmc/phosphor-dbus-interfaces/+/14595/. and https://gerrit.openbmc-project.xyz/#/c/openbmc/phosphor-dbus-interfaces/+/14718/ Change-Id: Id427d718b6fcc9b90dfb3bccb3b4cc665a107c46 Signed-off-by: Nagaraju Goruganti Signed-off-by: Deepak Kodihalli Signed-off-by: Ratan Gupta --- phosphor-ldap-config/ldap_configuration.cpp | 135 ++++++++++------------------ phosphor-ldap-config/ldap_configuration.hpp | 31 +++---- 2 files changed, 55 insertions(+), 111 deletions(-) diff --git a/phosphor-ldap-config/ldap_configuration.cpp b/phosphor-ldap-config/ldap_configuration.cpp index 4bbcbed..e84e0b9 100644 --- a/phosphor-ldap-config/ldap_configuration.cpp +++ b/phosphor-ldap-config/ldap_configuration.cpp @@ -24,17 +24,16 @@ using ConfigInfo = std::map; Config::Config(sdbusplus::bus::bus& bus, const char* path, const char* filePath, bool secureLDAP, std::string lDAPServerURI, std::string lDAPBindDN, std::string lDAPBaseDN, - std::string lDAPBindDNpassword, + std::string&& lDAPBindDNPassword, ldap_base::Config::SearchScope lDAPSearchScope, ldap_base::Config::Type lDAPType, ConfigMgr& parent) : ConfigIface(bus, path, true), - configFilePath(filePath), bus(bus), parent(parent) + secureLDAP(secureLDAP), configFilePath(filePath), + lDAPBindDNPassword(std::move(lDAPBindDNPassword)), bus(bus), parent(parent) { - ConfigIface::secureLDAP(secureLDAP); ConfigIface::lDAPServerURI(lDAPServerURI); ConfigIface::lDAPBindDN(lDAPBindDN); ConfigIface::lDAPBaseDN(lDAPBaseDN); - ConfigIface::lDAPBINDDNpassword(lDAPBindDNpassword); ConfigIface::lDAPSearchScope(lDAPSearchScope); ConfigIface::lDAPType(lDAPType); writeConfig(); @@ -81,9 +80,9 @@ void Config::writeConfig() confData << "uri " << lDAPServerURI() << "\n\n"; confData << "base " << lDAPBaseDN() << "\n\n"; confData << "binddn " << lDAPBindDN() << "\n"; - if (!lDAPBINDDNpassword().empty()) + if (!lDAPBindDNPassword.empty()) { - confData << "bindpw " << lDAPBINDDNpassword() << "\n"; + confData << "bindpw " << lDAPBindDNPassword << "\n"; isPwdTobeWritten = true; } confData << "\n"; @@ -101,7 +100,7 @@ void Config::writeConfig() } confData << "base passwd " << lDAPBaseDN() << "\n"; confData << "base shadow " << lDAPBaseDN() << "\n\n"; - if (secureLDAP() == true) + if (secureLDAP == true) { confData << "ssl on\n"; confData << "tls_reqcert allow\n"; @@ -166,33 +165,6 @@ void Config::writeConfig() return; } -bool Config::secureLDAP(bool value) -{ - bool val = false; - try - { - if (value == secureLDAP()) - { - return value; - } - - val = ConfigIface::secureLDAP(value); - writeConfig(); - parent.restartService(nslcdService); - } - catch (const InternalFailure& e) - { - throw; - } - catch (const std::exception& e) - { - log(e.what()); - elog(); - } - - return val; -} - std::string Config::lDAPServerURI(std::string value) { std::string val; @@ -202,13 +174,25 @@ std::string Config::lDAPServerURI(std::string value) { return value; } - if (!(ldap_is_ldap_url(value.c_str()) || - ldap_is_ldaps_url(value.c_str()))) + if (secureLDAP) { - log("Not a valid LDAP Server URI"), - entry("LDAPSERVERURI=%s", value.c_str()); - elog(Argument::ARGUMENT_NAME("lDAPServerURI"), - Argument::ARGUMENT_VALUE(value.c_str())); + if (!ldap_is_ldaps_url(value.c_str())) + { + log("bad LDAPS Server URI", + entry("LDAPSSERVERURI=%s", value.c_str())); + elog(Argument::ARGUMENT_NAME("lDAPServerURI"), + Argument::ARGUMENT_VALUE(value.c_str())); + } + } + else + { + if (!ldap_is_ldap_url(value.c_str())) + { + log("bad LDAP Server URI", + entry("LDAPSERVERURI=%s", value.c_str())); + elog(Argument::ARGUMENT_NAME("lDAPServerURI"), + Argument::ARGUMENT_VALUE(value.c_str())); + } } val = ConfigIface::lDAPServerURI(value); writeConfig(); @@ -223,7 +207,6 @@ std::string Config::lDAPServerURI(std::string value) log(e.what()); elog(); } - return val; } @@ -295,32 +278,6 @@ std::string Config::lDAPBaseDN(std::string value) return val; } -std::string Config::lDAPBINDDNpassword(std::string value) -{ - std::string val; - try - { - if (value == lDAPBINDDNpassword()) - { - return value; - } - - val = ConfigIface::lDAPBINDDNpassword(value); - writeConfig(); - parent.restartService(nslcdService); - } - catch (const InternalFailure& e) - { - throw; - } - catch (const std::exception& e) - { - log(e.what()); - elog(); - } - return val; -} - ldap_base::Config::SearchScope Config::lDAPSearchScope(ldap_base::Config::SearchScope value) { @@ -414,17 +371,26 @@ void ConfigMgr::deleteObject() } std::string - ConfigMgr::createConfig(bool secureLDAP, std::string lDAPServerURI, - std::string lDAPBindDN, std::string lDAPBaseDN, - std::string lDAPBINDDNpassword, + ConfigMgr::createConfig(std::string lDAPServerURI, std::string lDAPBindDN, + std::string lDAPBaseDN, + std::string lDAPBindDNPassword, ldap_base::Create::SearchScope lDAPSearchScope, ldap_base::Create::Type lDAPType) { - if (!(ldap_is_ldap_url(lDAPServerURI.c_str()) || - ldap_is_ldaps_url(lDAPServerURI.c_str()))) + bool secureLDAP = false; + + if (ldap_is_ldaps_url(lDAPServerURI.c_str())) + { + secureLDAP = true; + } + else if (ldap_is_ldap_url(lDAPServerURI.c_str())) + { + secureLDAP = false; + } + else { - log("Not a valid LDAP Server URI"), - entry("LDAPSERVERURI=%s", lDAPServerURI.c_str()); + log("bad LDAP Server URI", + entry("LDAPSERVERURI=%s", lDAPServerURI.c_str())); elog(Argument::ARGUMENT_NAME("lDAPServerURI"), Argument::ARGUMENT_VALUE(lDAPServerURI.c_str())); } @@ -463,8 +429,8 @@ std::string auto objPath = std::string(LDAP_CONFIG_DBUS_OBJ_PATH); configPtr = std::make_unique( - bus, objPath.c_str(), LDAP_CONFIG_FILE, secureLDAP, lDAPServerURI, - lDAPBindDN, lDAPBaseDN, lDAPBINDDNpassword, + bus, objPath.c_str(), configFilePath.c_str(), secureLDAP, lDAPServerURI, + lDAPBindDN, lDAPBaseDN, std::move(lDAPBindDNPassword), static_cast(lDAPSearchScope), static_cast(lDAPType), *this); @@ -548,17 +514,6 @@ void ConfigMgr::restore(const char* filePath) } } - // extract properties from configValues map - bool secureLDAP; - if (configValues["ssl"] == "on") - { - secureLDAP = true; - } - else - { - secureLDAP = false; - } - ldap_base::Create::SearchScope lDAPSearchScope; if (configValues["scope"] == "sub") { @@ -585,9 +540,9 @@ void ConfigMgr::restore(const char* filePath) } createConfig( - secureLDAP, std::move(configValues["uri"]), - std::move(configValues["binddn"]), std::move(configValues["base"]), - std::move(configValues["bindpw"]), lDAPSearchScope, lDAPType); + std::move(configValues["uri"]), std::move(configValues["binddn"]), + std::move(configValues["base"]), std::move(configValues["bindpw"]), + lDAPSearchScope, lDAPType); } catch (const InvalidArgument& e) { diff --git a/phosphor-ldap-config/ldap_configuration.hpp b/phosphor-ldap-config/ldap_configuration.hpp index 6601bd2..034aab8 100644 --- a/phosphor-ldap-config/ldap_configuration.hpp +++ b/phosphor-ldap-config/ldap_configuration.hpp @@ -53,7 +53,7 @@ class Config : public ConfigIface * @param[in] lDAPServerURI - LDAP URI of the server. * @param[in] lDAPBindDN - distinguished name with which to bind. * @param[in] lDAPBaseDN - distinguished name to use as search base. - * @param[in] lDAPBindDNpassword - credentials with which to bind. + * @param[in] lDAPBindDNPassword - credentials with which to bind. * @param[in] lDAPSearchScope - the search scope. * @param[in] lDAPType - Specifies the LDAP server type which can be AD or openLDAP. @@ -62,25 +62,17 @@ class Config : public ConfigIface Config(sdbusplus::bus::bus& bus, const char* path, const char* filePath, bool secureLDAP, std::string lDAPServerURI, std::string lDAPBindDN, - std::string lDAPBaseDN, std::string lDAPBindDNpassword, + std::string lDAPBaseDN, std::string&& lDAPBindDNPassword, ldap_base::Config::SearchScope lDAPSearchScope, ldap_base::Config::Type lDAPType, ConfigMgr& parent); using ConfigIface::lDAPBaseDN; using ConfigIface::lDAPBindDN; - using ConfigIface::lDAPBINDDNpassword; using ConfigIface::lDAPSearchScope; using ConfigIface::lDAPServerURI; using ConfigIface::lDAPType; - using ConfigIface::secureLDAP; using ConfigIface::setPropertyByName; - /** @brief Update the secure LDAP property. - * @param[in] value - secureLDAP value to be updated. - * @returns value of changed secureLDAP. - */ - bool secureLDAP(bool value) override; - /** @brief Update the Server URI property. * @param[in] value - lDAPServerURI value to be updated. * @returns value of changed lDAPServerURI. @@ -99,12 +91,6 @@ class Config : public ConfigIface */ std::string lDAPBaseDN(std::string value) override; - /** @brief Update the BindDN password property. - * @param[in] value - lDAPBINDDNpassword value to be updated. - * @returns value of changed lDAPBINDDNpassword. - */ - std::string lDAPBINDDNpassword(std::string value) override; - /** @brief Update the Search scope property. * @param[in] value - lDAPSearchScope value to be updated. * @returns value of changed lDAPSearchScope. @@ -122,8 +108,11 @@ class Config : public ConfigIface */ void delete_() override; + bool secureLDAP; + private: std::string configFilePath{}; + std::string lDAPBindDNPassword{}; /** @brief Persistent sdbusplus D-Bus bus connection. */ sdbusplus::bus::bus& bus; @@ -174,20 +163,19 @@ class ConfigMgr : public CreateIface /** @brief concrete implementation of the pure virtual funtion xyz.openbmc_project.User.Ldap.Create.createConfig. - * @param[in] secureLDAP - Specifies whether to use SSL or not. * @param[in] lDAPServerURI - LDAP URI of the server. * @param[in] lDAPBindDN - distinguished name with which bind to bind to the directory server for lookups. * @param[in] lDAPBaseDN - distinguished name to use as search base. - * @param[in] lDAPBindDNpassword - credentials with which to bind. + * @param[in] lDAPBindDNPassword - credentials with which to bind. * @param[in] lDAPSearchScope - the search scope. * @param[in] lDAPType - Specifies the LDAP server type which can be AD or openLDAP. * @returns the object path of the D-Bus object created. */ - std::string createConfig(bool secureLDAP, std::string lDAPServerURI, - std::string lDAPBindDN, std::string lDAPBaseDN, - std::string lDAPBindDNpassword, + std::string createConfig(std::string lDAPServerURI, std::string lDAPBindDN, + std::string lDAPBaseDN, + std::string lDAPBindDNPassword, ldap_base::Create::SearchScope lDAPSearchScope, ldap_base::Create::Type lDAPType) override; @@ -206,6 +194,7 @@ class ConfigMgr : public CreateIface void deleteObject(); private: + std::string configFilePath{}; /** @brief Persistent sdbusplus D-Bus bus connection. */ sdbusplus::bus::bus& bus; -- cgit v1.2.1