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>

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<Key, Val>;
 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<level::ERR>(e.what());
-        elog<InternalFailure>();
-    }
-
-    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<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 @@ std::string Config::lDAPServerURI(std::string value)
         log<level::ERR>(e.what());
         elog<InternalFailure>();
     }
-
     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<level::ERR>(e.what());
-        elog<InternalFailure>();
-    }
-    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<level::ERR>("Not a valid LDAP Server URI"),
-            entry("LDAPSERVERURI=%s", lDAPServerURI.c_str());
+        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 @@ std::string
 
     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 @@ 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;