Create the default object for openldap and AD.
This commit introduces the following functionalities
=> Default AD and openldap config object would always be there.
=> User should not be able to change the type of the ldap
once it is created.
This change is to align with redfish sehema
(https://redfish.dmtf.org/schemas/AccountService.v1_4_0.json),
In the schema AD and LDAP is a property which user can PATCH,
Now with the current code which doesn't have the default config
so for the PATCH, We were forcing the user to give all the
properties and then create the object which is against the
PATCH semantics.
TestedBy: Unit tested
Default Object gets created when service starts.
change of ldap type gets the error back.
Signed-off-by: Ratan Gupta <ratagupt@linux.vnet.ibm.com>
Change-Id: I0ce951a13ee525df022fb0716f0aea10d1909781
diff --git a/phosphor-ldap-config/ldap_config.cpp b/phosphor-ldap-config/ldap_config.cpp
index d10a711..cfa825c 100644
--- a/phosphor-ldap-config/ldap_config.cpp
+++ b/phosphor-ldap-config/ldap_config.cpp
@@ -20,6 +20,8 @@
using namespace sdbusplus::xyz::openbmc_project::Common::Error;
namespace fs = std::filesystem;
using Argument = xyz::openbmc_project::Common::InvalidArgument;
+using NotAllowed = sdbusplus::xyz::openbmc_project::Common::Error::NotAllowed;
+using NotAllowedArgument = xyz::openbmc_project::Common::NotAllowed;
using Line = std::string;
using Key = std::string;
@@ -380,28 +382,8 @@
ConfigIface::Type Config::lDAPType(ConfigIface::Type value)
{
- ConfigIface::Type val;
- try
- {
- if (value == lDAPType())
- {
- return value;
- }
-
- val = ConfigIface::lDAPType(value);
- writeConfig();
- parent.startOrStopService(nslcdService, enabled());
- }
- catch (const InternalFailure& e)
- {
- throw;
- }
- catch (const std::exception& e)
- {
- log<level::ERR>(e.what());
- elog<InternalFailure>();
- }
- return val;
+ elog<NotAllowed>(NotAllowedArgument::REASON("ReadOnly Property"));
+ return lDAPType();
}
bool Config::enabled(bool value)
diff --git a/phosphor-ldap-config/ldap_config_mgr.cpp b/phosphor-ldap-config/ldap_config_mgr.cpp
index 0a79ef2..a60e8f8 100644
--- a/phosphor-ldap-config/ldap_config_mgr.cpp
+++ b/phosphor-ldap-config/ldap_config_mgr.cpp
@@ -75,7 +75,8 @@
void ConfigMgr::deleteObject()
{
- configPtr.reset(nullptr);
+ // TODO Not needed the delete functionality.
+ // will do in later commit
}
std::string ConfigMgr::createConfig(
@@ -125,173 +126,60 @@
Argument::ARGUMENT_VALUE(lDAPBaseDN.c_str()));
}
- // With current implementation we support only one LDAP server.
- deleteObject();
+ // With current implementation we support only two default LDAP server.
+ // which will be always there but when the support comes for additional
+ // account providers then the create config would be used to create the
+ // additional config.
- auto objPath = std::string(LDAP_CONFIG_DBUS_OBJ_PATH);
- configPtr = std::make_unique<Config>(
- bus, objPath.c_str(), configFilePath.c_str(), tlsCacertFile.c_str(),
- secureLDAP, lDAPServerURI, lDAPBindDN, lDAPBaseDN,
- std::move(lDAPBindDNPassword),
- static_cast<ConfigIface::SearchScope>(lDAPSearchScope),
- static_cast<ConfigIface::Type>(lDAPType), false, groupNameAttribute,
- userNameAttribute, *this);
+ std::string objPath;
+ if (static_cast<ConfigIface::Type>(lDAPType) == ConfigIface::Type::OpenLdap)
+ {
+ openLDAPConfigPtr.reset(nullptr);
+ objPath = openLDAPDbusObjectPath;
+ openLDAPConfigPtr = std::make_unique<Config>(
+ bus, objPath.c_str(), configFilePath.c_str(), tlsCacertFile.c_str(),
+ secureLDAP, lDAPServerURI, lDAPBindDN, lDAPBaseDN,
+ std::move(lDAPBindDNPassword),
+ static_cast<ConfigIface::SearchScope>(lDAPSearchScope),
+ static_cast<ConfigIface::Type>(lDAPType), false, groupNameAttribute,
+ userNameAttribute, *this);
+ }
+ else
+ {
+ ADConfigPtr.reset(nullptr);
+ objPath = ADDbusObjectPath;
+ ADConfigPtr = std::make_unique<Config>(
+ bus, objPath.c_str(), configFilePath.c_str(), tlsCacertFile.c_str(),
+ secureLDAP, lDAPServerURI, lDAPBindDN, lDAPBaseDN,
+ std::move(lDAPBindDNPassword),
+ static_cast<ConfigIface::SearchScope>(lDAPSearchScope),
+ static_cast<ConfigIface::Type>(lDAPType), false, groupNameAttribute,
+ userNameAttribute, *this);
+ }
restartService(nscdService);
return objPath;
}
-void ConfigMgr::restore(const char* filePath)
+void ConfigMgr::createDefaultObjects()
{
- if (!fs::exists(filePath))
+ if (!openLDAPConfigPtr)
{
- log<level::ERR>("Config file doesn't exists",
- entry("LDAP_CONFIG_FILE=%s", configFilePath.c_str()));
- return;
+ openLDAPConfigPtr = std::make_unique<Config>(
+ bus, openLDAPDbusObjectPath.c_str(), configFilePath.c_str(),
+ tlsCacertFile.c_str(), false, "", "", "", "",
+ ConfigIface::SearchScope::sub, ConfigIface::Type::OpenLdap, false,
+ "", "", *this);
}
-
- ConfigInfo configValues;
- try
+ if (!ADConfigPtr)
{
- std::fstream stream(filePath, std::fstream::in);
- Line line;
- // read characters from stream and places them into line
- while (std::getline(stream, line))
- {
- // remove leading and trailing extra spaces
- auto firstScan = line.find_first_not_of(' ');
- auto first =
- (firstScan == std::string::npos ? line.length() : firstScan);
- auto last = line.find_last_not_of(' ');
- line = line.substr(first, last - first + 1);
- // reduce multiple spaces between two words to a single space
- auto pred = [](char a, char b) {
- return (a == b && a == ' ') ? true : false;
- };
-
- auto lastPos = std::unique(line.begin(), line.end(), pred);
-
- line.erase(lastPos, line.end());
-
- // Ignore if line is empty or starts with '#'
- if (line.empty() || line.at(0) == '#')
- {
- continue;
- }
-
- Key key;
- std::istringstream isLine(line);
- // extract characters from isLine and stores them into
- // key until the delimitation character ' ' is found.
- // If the delimiter is found, it is extracted and discarded
- // the next input operation will begin after it.
- if (std::getline(isLine, key, ' '))
- {
- Val value;
- // extract characters after delimitation character ' '
- if (std::getline(isLine, value, ' '))
- {
- // skip line if it starts with "base shadow" or
- // "base passwd" because we would have 3 entries
- // ("base lDAPBaseDN" , "base passwd lDAPBaseDN" and
- // "base shadow lDAPBaseDN") for the property "lDAPBaseDN",
- // one is enough to restore it.
-
- if ((key == "base") &&
- (value == "passwd" || value == "shadow"))
- {
- continue;
- }
-
- // if config type is AD "map group" entry would be add to
- // the map configValues. For OpenLdap config file no map
- // entry would be there.
- if ((key == "map") && (value == "passwd"))
- {
- key = key + "_" + value;
- if (std::getline(isLine, value, ' '))
- {
- key += "_" + value;
- }
- std::getline(isLine, value, ' ');
- }
- configValues[key] = value;
- }
- }
- }
-
- CreateIface::SearchScope lDAPSearchScope;
- if (configValues["scope"] == "sub")
- {
- lDAPSearchScope = CreateIface::SearchScope::sub;
- }
- else if (configValues["scope"] == "one")
- {
- lDAPSearchScope = CreateIface::SearchScope::one;
- }
- else
- {
- lDAPSearchScope = CreateIface::SearchScope::base;
- }
-
- CreateIface::Type lDAPType;
- // If the file is having a line which starts with "map group"
- if (configValues["map"] == "group")
- {
- lDAPType = CreateIface::Type::ActiveDirectory;
- }
- else
- {
- lDAPType = CreateIface::Type::OpenLdap;
- }
-
- // Don't create the config object if either of the field is empty.
- if (configValues["uri"] == "" || configValues["binddn"] == "" ||
- configValues["base"] == "")
- {
- log<level::INFO>(
- "LDAP config parameter value missing",
- entry("URI=%s", configValues["uri"].c_str()),
- entry("BASEDN=%s", configValues["base"].c_str()),
- entry("BINDDN=%s", configValues["binddn"].c_str()));
- return;
- }
-
- createConfig(std::move(configValues["uri"]),
- std::move(configValues["binddn"]),
- std::move(configValues["base"]),
- std::move(configValues["bindpw"]), lDAPSearchScope,
- lDAPType, std::move(configValues["map_passwd_uid"]),
- std::move(configValues["map_passwd_gidNumber"]));
-
- // Get the enabled property value from the persistent location
- if (!deserialize(dbusPersistentPath, *configPtr))
- {
- log<level::INFO>(
- "Deserialization Failed, continue with service disable");
- }
- }
- catch (const InvalidArgument& e)
- {
- // Don't throw - we don't want to create a D-Bus
- // object upon finding empty values in config, as
- // this can be a default config.
- }
- catch (const NoCACertificate& e)
- {
- // Don't throw - we don't want to create a D-Bus
- // object upon finding "ssl on" without having tls_cacertFile in place,
- // as this can be a default config.
- }
- catch (const InternalFailure& e)
- {
- throw;
- }
- catch (const std::exception& e)
- {
- log<level::ERR>(e.what());
- elog<InternalFailure>();
+ ADConfigPtr = std::make_unique<Config>(
+ bus, ADDbusObjectPath.c_str(), configFilePath.c_str(),
+ tlsCacertFile.c_str(), false, "", "", "", "",
+ ConfigIface::SearchScope::sub, ConfigIface::Type::ActiveDirectory,
+ false, "", "", *this);
}
}
+
} // namespace ldap
} // namespace phosphor
diff --git a/phosphor-ldap-config/ldap_config_mgr.hpp b/phosphor-ldap-config/ldap_config_mgr.hpp
index b30d98c..a3895ae 100644
--- a/phosphor-ldap-config/ldap_config_mgr.hpp
+++ b/phosphor-ldap-config/ldap_config_mgr.hpp
@@ -18,6 +18,10 @@
static constexpr auto defaultNslcdFile = "nslcd.conf.default";
static constexpr auto nsSwitchFile = "nsswitch.conf";
+static auto openLDAPDbusObjectPath =
+ std::string(LDAP_CONFIG_ROOT) + "/openldap";
+static auto ADDbusObjectPath =
+ std::string(LDAP_CONFIG_ROOT) + "/active_directory";
using namespace phosphor::logging;
using namespace sdbusplus::xyz::openbmc_project::Common::Error;
@@ -53,17 +57,6 @@
dbusPersistentPath(dbusPersistentPath), configFilePath(filePath),
bus(bus)
{
- try
- {
- restore(configFilePath.c_str());
- emit_object_added();
- }
- catch (const std::exception& e)
- {
- configPtr.reset(nullptr);
- log<level::ERR>(e.what());
- elog<InternalFailure>();
- }
}
/** @brief concrete implementation of the pure virtual funtion
@@ -110,6 +103,10 @@
*/
void deleteObject();
+ /* Create the default active directory and the openldap config
+ * objects. */
+ virtual void createDefaultObjects();
+
/* ldap service enabled property would be saved under
* this path.
*/
@@ -122,13 +119,17 @@
/** @brief Persistent sdbusplus D-Bus bus connection. */
sdbusplus::bus::bus& bus;
- /** @brief Pointer to a Config D-Bus object */
- std::unique_ptr<Config> configPtr = nullptr;
+ /* Below two config objects are default, which will always be there */
- /** @brief Populate existing config into D-Bus properties
- * @param[in] filePath - LDAP config file path
- */
- virtual void restore(const char* filePath);
+ /* if need arises then we can have below map for additional account
+ * providers we need to create sub class of Config which will implement the
+ * delete interface as the default objects will not implement the delete
+ * std::map<std::string, std::unique_ptr<NewConfig>> AdditionalProviders*/
+
+ /** @brief Pointer to a openLDAP Config D-Bus object */
+ std::unique_ptr<Config> openLDAPConfigPtr = nullptr;
+ /** @brief Pointer to a AD Config D-Bus object */
+ std::unique_ptr<Config> ADConfigPtr = nullptr;
};
} // namespace ldap
} // namespace phosphor
diff --git a/phosphor-ldap-config/main.cpp b/phosphor-ldap-config/main.cpp
index c6eaa5a..e0ac638 100644
--- a/phosphor-ldap-config/main.cpp
+++ b/phosphor-ldap-config/main.cpp
@@ -27,6 +27,7 @@
phosphor::ldap::ConfigMgr mgr(bus, LDAP_CONFIG_ROOT, LDAP_CONFIG_FILE,
LDAP_CONF_PERSIST_PATH, TLS_CACERT_FILE);
+ mgr.createDefaultObjects();
bus.request_name(LDAP_CONFIG_BUSNAME);
diff --git a/test/ldap_config_test.cpp b/test/ldap_config_test.cpp
index 05cea20..74c8171 100644
--- a/test/ldap_config_test.cpp
+++ b/test/ldap_config_test.cpp
@@ -71,22 +71,32 @@
}
MOCK_METHOD1(restartService, void(const std::string& service));
MOCK_METHOD1(stopService, void(const std::string& service));
- std::unique_ptr<Config>& getConfigPtr()
+ std::unique_ptr<Config>& getOpenLdapConfigPtr()
{
- return configPtr;
+ return openLDAPConfigPtr;
}
std::string configBindPassword()
{
- return getConfigPtr()->lDAPBindPassword;
+ return getADConfigPtr()->lDAPBindPassword;
}
- void restore(const char* filePath)
+ std::unique_ptr<Config>& getADConfigPtr()
{
- phosphor::ldap::ConfigMgr::restore(filePath);
+ return ADConfigPtr;
+ }
+ void restore()
+ {
+ // TODO enable it in later commit.
+ // phosphor::ldap::ConfigMgr::restore();
return;
}
+ void createDefaultObjects()
+ {
+ phosphor::ldap::ConfigMgr::createDefaultObjects();
+ }
+
friend class TestLDAPConfig;
};
@@ -105,32 +115,65 @@
MockConfigMgr manager(bus, LDAP_CONFIG_ROOT, configFilePath.c_str(),
dbusPersistentFilePath.c_str(),
tlsCacertfile.c_str());
+
+ EXPECT_CALL(manager, stopService("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",
"MyLdap12", ldap_base::Create::SearchScope::sub,
ldap_base::Create::Type::ActiveDirectory, "uid", "gid");
- manager.getConfigPtr()->enabled(true);
+ manager.getADConfigPtr()->enabled(true);
EXPECT_TRUE(fs::exists(configFilePath));
- EXPECT_EQ(manager.getConfigPtr()->lDAPServerURI(), "ldap://9.194.251.136/");
- EXPECT_EQ(manager.getConfigPtr()->lDAPBindDN(), "cn=Users,dc=com");
- EXPECT_EQ(manager.getConfigPtr()->lDAPBaseDN(), "cn=Users,dc=corp");
- EXPECT_EQ(manager.getConfigPtr()->lDAPSearchScope(),
+ EXPECT_EQ(manager.getADConfigPtr()->lDAPServerURI(),
+ "ldap://9.194.251.136/");
+ EXPECT_EQ(manager.getADConfigPtr()->lDAPBindDN(), "cn=Users,dc=com");
+ EXPECT_EQ(manager.getADConfigPtr()->lDAPBaseDN(), "cn=Users,dc=corp");
+ EXPECT_EQ(manager.getADConfigPtr()->lDAPSearchScope(),
ldap_base::Config::SearchScope::sub);
- EXPECT_EQ(manager.getConfigPtr()->lDAPType(),
+ EXPECT_EQ(manager.getADConfigPtr()->lDAPType(),
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.getADConfigPtr()->userNameAttribute(), "uid");
+ EXPECT_EQ(manager.getADConfigPtr()->groupNameAttribute(), "gid");
+ EXPECT_EQ(manager.getADConfigPtr()->lDAPBindDNPassword(), "");
EXPECT_EQ(manager.configBindPassword(), "MyLdap12");
// change the password
- manager.getConfigPtr()->lDAPBindDNPassword("MyLdap14");
- EXPECT_EQ(manager.getConfigPtr()->lDAPBindDNPassword(), "");
+ manager.getADConfigPtr()->lDAPBindDNPassword("MyLdap14");
+ EXPECT_EQ(manager.getADConfigPtr()->lDAPBindDNPassword(), "");
EXPECT_EQ(manager.configBindPassword(), "MyLdap14");
}
+TEST_F(TestLDAPConfig, testDefaultObject)
+{
+ auto configFilePath = std::string(dir.c_str()) + "/" + ldapconfFile;
+ auto tlsCacertfile = std::string(dir.c_str()) + "/" + tslCacertFile;
+ auto dbusPersistentFilePath =
+ std::string(dir.c_str()) + "/" + dbusPersistFile;
+
+ if (fs::exists(configFilePath))
+ {
+ fs::remove(configFilePath);
+ }
+ EXPECT_FALSE(fs::exists(configFilePath));
+
+ MockConfigMgr manager(bus, LDAP_CONFIG_ROOT, configFilePath.c_str(),
+ dbusPersistentFilePath.c_str(),
+ tlsCacertfile.c_str());
+
+ EXPECT_CALL(manager, stopService("nslcd.service")).Times(2);
+
+ manager.createDefaultObjects();
+
+ EXPECT_NE(nullptr, manager.getADConfigPtr());
+ EXPECT_NE(nullptr, manager.getOpenLdapConfigPtr());
+ EXPECT_EQ(manager.getADConfigPtr()->lDAPType(),
+ ldap_base::Config::Type::ActiveDirectory);
+ EXPECT_EQ(manager.getOpenLdapConfigPtr()->lDAPType(),
+ ldap_base::Config::Type::OpenLdap);
+}
+/*
TEST_F(TestLDAPConfig, testRestores)
{
auto configFilePath = std::string(dir.c_str()) + "/" + ldapconfFile;
@@ -399,5 +442,6 @@
ldap_base::Config::Type::OpenLdap);
delete managerPtr;
}
+*/
} // namespace ldap
} // namespace phosphor