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 <ngorugan@in.ibm.com>
Signed-off-by: Deepak Kodihalli <dkodihal@in.ibm.com>
Signed-off-by: Ratan Gupta <ratagupt@linux.vnet.ibm.com>
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 @@
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 @@
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 @@
}
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 @@
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<level::ERR>(e.what());
- elog<InternalFailure>();
- }
-
- return val;
-}
-
std::string Config::lDAPServerURI(std::string value)
{
std::string val;
@@ -202,13 +174,25 @@
{
return value;
}
- if (!(ldap_is_ldap_url(value.c_str()) ||
- ldap_is_ldaps_url(value.c_str())))
+ if (secureLDAP)
{
- log<level::ERR>("Not a valid LDAP Server URI"),
- entry("LDAPSERVERURI=%s", value.c_str());
- elog<InvalidArgument>(Argument::ARGUMENT_NAME("lDAPServerURI"),
- Argument::ARGUMENT_VALUE(value.c_str()));
+ if (!ldap_is_ldaps_url(value.c_str()))
+ {
+ log<level::ERR>("bad LDAPS Server URI",
+ entry("LDAPSSERVERURI=%s", value.c_str()));
+ elog<InvalidArgument>(Argument::ARGUMENT_NAME("lDAPServerURI"),
+ Argument::ARGUMENT_VALUE(value.c_str()));
+ }
+ }
+ else
+ {
+ if (!ldap_is_ldap_url(value.c_str()))
+ {
+ log<level::ERR>("bad LDAP Server URI",
+ entry("LDAPSERVERURI=%s", value.c_str()));
+ elog<InvalidArgument>(Argument::ARGUMENT_NAME("lDAPServerURI"),
+ Argument::ARGUMENT_VALUE(value.c_str()));
+ }
}
val = ConfigIface::lDAPServerURI(value);
writeConfig();
@@ -223,7 +207,6 @@
log<level::ERR>(e.what());
elog<InternalFailure>();
}
-
return val;
}
@@ -295,32 +278,6 @@
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<level::ERR>(e.what());
- elog<InternalFailure>();
- }
- return val;
-}
-
ldap_base::Config::SearchScope
Config::lDAPSearchScope(ldap_base::Config::SearchScope value)
{
@@ -414,17 +371,26 @@
}
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()))
{
- log<level::ERR>("Not a valid LDAP Server URI"),
- entry("LDAPSERVERURI=%s", lDAPServerURI.c_str());
+ secureLDAP = true;
+ }
+ else if (ldap_is_ldap_url(lDAPServerURI.c_str()))
+ {
+ secureLDAP = false;
+ }
+ else
+ {
+ log<level::ERR>("bad LDAP Server URI",
+ entry("LDAPSERVERURI=%s", lDAPServerURI.c_str()));
elog<InvalidArgument>(Argument::ARGUMENT_NAME("lDAPServerURI"),
Argument::ARGUMENT_VALUE(lDAPServerURI.c_str()));
}
@@ -463,8 +429,8 @@
auto objPath = std::string(LDAP_CONFIG_DBUS_OBJ_PATH);
configPtr = std::make_unique<Config>(
- 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<ldap_base::Config::SearchScope>(lDAPSearchScope),
static_cast<ldap_base::Config::Type>(lDAPType), *this);
@@ -548,17 +514,6 @@
}
}
- // 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 @@
}
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 @@
* @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 @@
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 @@
*/
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 @@
*/
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 @@
/** @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 @@
void deleteObject();
private:
+ std::string configFilePath{};
/** @brief Persistent sdbusplus D-Bus bus connection. */
sdbusplus::bus::bus& bus;