LDAP Config: Extend the support to change the BindDNPassword
Before this commit we don't allow the user to change the bind
DN password as our REST API was the mirror of the D-bus API.
Now with the introduction of Redfish, where we have to give the
support for changing the bind dn password.
With this fix, set property on the d-bus object would update the
underlying ldap config file but wouldn't update the D-bus object due
to security issue.
Signed-off-by: Ratan Gupta <ratagupt@linux.vnet.ibm.com>
Change-Id: I6072820185cd540fe44850b90a4f6c256c44471c
diff --git a/phosphor-ldap-config/ldap_configuration.cpp b/phosphor-ldap-config/ldap_configuration.cpp
index 466b72d..77726ee 100644
--- a/phosphor-ldap-config/ldap_configuration.cpp
+++ b/phosphor-ldap-config/ldap_configuration.cpp
@@ -33,8 +33,9 @@
std::string userNameAttr, std::string groupNameAttr,
ConfigMgr& parent) :
Ifaces(bus, path, true),
- secureLDAP(secureLDAP), configFilePath(filePath), tlsCacertFile(caCertFile),
- lDAPBindDNPassword(std::move(lDAPBindDNPassword)), bus(bus), parent(parent)
+ secureLDAP(secureLDAP), lDAPBindPassword(std::move(lDAPBindDNPassword)),
+ configFilePath(filePath), tlsCacertFile(caCertFile), bus(bus),
+ parent(parent)
{
ConfigIface::lDAPServerURI(lDAPServerURI);
ConfigIface::lDAPBindDN(lDAPBindDN);
@@ -44,6 +45,7 @@
EnableIface::enabled(lDAPServiceEnabled);
ConfigIface::userNameAttribute(userNameAttr);
ConfigIface::groupNameAttribute(groupNameAttr);
+ // Don't update the bindDN password under ConfigIface::
writeConfig();
// Emit deferred signal.
this->emit_object_added();
@@ -87,9 +89,9 @@
confData << "uri " << lDAPServerURI() << "\n\n";
confData << "base " << lDAPBaseDN() << "\n\n";
confData << "binddn " << lDAPBindDN() << "\n";
- if (!lDAPBindDNPassword.empty())
+ if (!lDAPBindPassword.empty())
{
- confData << "bindpw " << lDAPBindDNPassword << "\n";
+ confData << "bindpw " << lDAPBindPassword << "\n";
isPwdTobeWritten = true;
}
confData << "\n";
@@ -194,6 +196,29 @@
return;
}
+std::string Config::lDAPBindDNPassword(std::string value)
+{
+ // Don't update the D-bus object, this is just to
+ // facilitate if user wants to change the bind dn password
+ // once d-bus object gets created.
+ lDAPBindPassword = value;
+ try
+ {
+ writeConfig();
+ parent.startOrStopService(nslcdService, enabled());
+ }
+ catch (const InternalFailure& e)
+ {
+ throw;
+ }
+ catch (const std::exception& e)
+ {
+ log<level::ERR>(e.what());
+ elog<InternalFailure>();
+ }
+ return value;
+}
+
std::string Config::lDAPServerURI(std::string value)
{
std::string val;
diff --git a/phosphor-ldap-config/ldap_configuration.hpp b/phosphor-ldap-config/ldap_configuration.hpp
index 976aac6..d4fe5b7 100644
--- a/phosphor-ldap-config/ldap_configuration.hpp
+++ b/phosphor-ldap-config/ldap_configuration.hpp
@@ -31,6 +31,7 @@
sdbusplus::xyz::openbmc_project::User::Ldap::server::Create>;
class ConfigMgr;
+class MockConfigMgr;
/** @class Config
* @brief Configuration for LDAP.
@@ -81,6 +82,7 @@
using ConfigIface::groupNameAttribute;
using ConfigIface::lDAPBaseDN;
using ConfigIface::lDAPBindDN;
+ using ConfigIface::lDAPBindDNPassword;
using ConfigIface::lDAPSearchScope;
using ConfigIface::lDAPServerURI;
using ConfigIface::lDAPType;
@@ -137,6 +139,12 @@
*/
std::string groupNameAttribute(std::string value) override;
+ /** @brief Update the BindDNPasword property.
+ * @param[in] value - lDAPBindDNPassword value to be updated.
+ * @returns value of changed lDAPBindDNPassword.
+ */
+ std::string lDAPBindDNPassword(std::string value) override;
+
/** @brief Delete this D-bus object.
*/
void delete_() override;
@@ -144,9 +152,9 @@
bool secureLDAP;
private:
+ std::string lDAPBindPassword{};
std::string configFilePath{};
std::string tlsCacertFile{};
- std::string lDAPBindDNPassword{};
/** @brief Persistent sdbusplus D-Bus bus connection. */
sdbusplus::bus::bus& bus;
@@ -157,6 +165,8 @@
/** @brief reference to config manager object */
ConfigMgr& parent;
+
+ friend class MockConfigMgr;
};
/** @class ConfigMgr
diff --git a/test/ldap_config_test.cpp b/test/ldap_config_test.cpp
index f7962f7..f9cf386 100644
--- a/test/ldap_config_test.cpp
+++ b/test/ldap_config_test.cpp
@@ -75,6 +75,11 @@
return configPtr;
}
+ std::string configBindPassword()
+ {
+ return getConfigPtr()->lDAPBindPassword;
+ }
+
void restore(const char* filePath)
{
phosphor::ldap::ConfigMgr::restore(filePath);
@@ -99,7 +104,7 @@
MockConfigMgr manager(bus, LDAP_CONFIG_ROOT, configFilePath.c_str(),
dbusPersistentFilePath.c_str(),
tlsCacertfile.c_str());
- EXPECT_CALL(manager, restartService("nslcd.service")).Times(1);
+ EXPECT_CALL(manager, restartService("nslcd.service")).Times(2);
EXPECT_CALL(manager, restartService("nscd.service")).Times(1);
manager.createConfig(
"ldap://9.194.251.136/", "cn=Users,dc=com", "cn=Users,dc=corp",
@@ -117,6 +122,12 @@
ldap_base::Config::Type::ActiveDirectory);
EXPECT_EQ(manager.getConfigPtr()->userNameAttribute(), "uid");
EXPECT_EQ(manager.getConfigPtr()->groupNameAttribute(), "gid");
+ EXPECT_EQ(manager.getConfigPtr()->lDAPBindDNPassword(), "");
+ EXPECT_EQ(manager.configBindPassword(), "MyLdap12");
+ // change the password
+ manager.getConfigPtr()->lDAPBindDNPassword("MyLdap14");
+ EXPECT_EQ(manager.getConfigPtr()->lDAPBindDNPassword(), "");
+ EXPECT_EQ(manager.configBindPassword(), "MyLdap14");
}
TEST_F(TestLDAPConfig, testRestores)
@@ -162,6 +173,8 @@
ldap_base::Config::Type::ActiveDirectory);
EXPECT_EQ(managerPtr->getConfigPtr()->userNameAttribute(), "uid");
EXPECT_EQ(managerPtr->getConfigPtr()->groupNameAttribute(), "gid");
+ EXPECT_EQ(managerPtr->getConfigPtr()->lDAPBindDNPassword(), "");
+ EXPECT_EQ(managerPtr->configBindPassword(), "MyLdap12");
delete managerPtr;
}