updateGroupsAndPriv: fix bugs and add tests
This commit adds a unit test for the |updateGroupsAndPriv| function.
Thanks to the test case, I found a bug that the exsting codes don't
updat the groups and privilege of the user after updating.
Added several neccessary functions to add unit test and fix bugs.
Tested: unit test passed.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ifcf88505f10b6bfdcca2de31a29ce055153463e8
diff --git a/test/user_mgr_test.cpp b/test/user_mgr_test.cpp
index 6264182..f2a9dd7 100644
--- a/test/user_mgr_test.cpp
+++ b/test/user_mgr_test.cpp
@@ -261,6 +261,9 @@
ON_CALL(*this, getIpmiUsersCount).WillByDefault(testing::Return(0));
ON_CALL(*this, executeUserRename).WillByDefault(testing::Return());
+
+ ON_CALL(*this, executeUserModify(testing::_, testing::_, testing::_))
+ .WillByDefault(testing::Return());
}
~UserMgrInTest() override
@@ -278,6 +281,10 @@
MOCK_METHOD(void, executeUserRename, (const char*, const char*),
(override));
+ MOCK_METHOD(void, executeUserModify, (const char*, const char*, bool),
+ (override));
+
+ protected:
static sdbusplus::bus_t busInTest;
std::string tempPamConfigFile;
};
@@ -509,6 +516,40 @@
EXPECT_THROW(
UserMgr::executeUserRename("user0", "user1"),
sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure);
+ EXPECT_THROW(
+ UserMgr::executeUserModify("user0", "ssh", true),
+ sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure);
+}
+
+TEST_F(UserMgrInTest, UpdateGroupsAndPrivOnSuccess)
+{
+ std::string username = "user001";
+ EXPECT_NO_THROW(
+ UserMgr::createUser(username, {"redfish", "ssh"}, "priv-user", true));
+ EXPECT_NO_THROW(
+ updateGroupsAndPriv(username, {"ipmi", "ssh"}, "priv-admin"));
+ UserInfoMap userInfo = getUserInfo(username);
+ EXPECT_EQ(std::get<Privilege>(userInfo["UserPrivilege"]), "priv-admin");
+ EXPECT_THAT(std::get<GroupList>(userInfo["UserGroups"]),
+ testing::UnorderedElementsAre("ipmi", "ssh"));
+ EXPECT_EQ(std::get<UserEnabled>(userInfo["UserEnabled"]), true);
+ EXPECT_NO_THROW(UserMgr::deleteUser(username));
+}
+
+TEST_F(UserMgrInTest,
+ UpdateGroupsAndPrivThrowsInternalFailureIfExecuteUserModifyFail)
+{
+ std::string username = "user001";
+ EXPECT_NO_THROW(
+ UserMgr::createUser(username, {"redfish", "ssh"}, "priv-user", true));
+ EXPECT_CALL(*this, executeUserModify(testing::StrEq(username), testing::_,
+ testing::_))
+ .WillOnce(testing::Throw(
+ sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure()));
+ EXPECT_THROW(
+ updateGroupsAndPriv(username, {"ipmi", "ssh"}, "priv-admin"),
+ sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure);
+ EXPECT_NO_THROW(UserMgr::deleteUser(username));
}
} // namespace user
diff --git a/user_mgr.cpp b/user_mgr.cpp
index 8656e66..7bc86b3 100644
--- a/user_mgr.cpp
+++ b/user_mgr.cpp
@@ -365,7 +365,7 @@
}
void UserMgr::updateGroupsAndPriv(const std::string& userName,
- const std::vector<std::string>& groupNames,
+ std::vector<std::string> groupNames,
const std::string& priv)
{
throwForInvalidPrivilege(priv);
@@ -402,8 +402,7 @@
}
try
{
- executeCmd("/usr/sbin/usermod", userName.c_str(), "-G", groups.c_str(),
- "-s", (sshRequested ? "/bin/sh" : "/bin/nologin"));
+ executeUserModify(userName.c_str(), groups.c_str(), sshRequested);
}
catch (const InternalFailure& e)
{
@@ -413,6 +412,9 @@
log<level::INFO>("User groups / privilege updated successfully",
entry("USER_NAME=%s", userName.c_str()));
+ std::sort(groupNames.begin(), groupNames.end());
+ usersList[userName]->setUserGroups(groupNames);
+ usersList[userName]->setUserPrivilege(priv);
return;
}
@@ -1333,5 +1335,12 @@
newHomeDir.c_str(), "-m");
}
+void UserMgr::executeUserModify(const char* userName, const char* newGroups,
+ bool sshRequested)
+{
+ executeCmd("/usr/sbin/usermod", userName, "-G", newGroups, "-s",
+ (sshRequested ? "/bin/sh" : "/bin/nologin"));
+}
+
} // namespace user
} // namespace phosphor
diff --git a/user_mgr.hpp b/user_mgr.hpp
index b057bf0..a535cc2 100644
--- a/user_mgr.hpp
+++ b/user_mgr.hpp
@@ -160,7 +160,7 @@
* @param[in] priv - Privilege to be updated.
*/
void updateGroupsAndPriv(const std::string& userName,
- const std::vector<std::string>& groups,
+ std::vector<std::string> groups,
const std::string& priv);
/** @brief Update user enabled state.
@@ -316,6 +316,9 @@
virtual void executeUserRename(const char* userName,
const char* newUserName);
+ virtual void executeUserModify(const char* userName, const char* newGroups,
+ bool sshRequested);
+
/** @brief check for valid privielge
* method to check valid privilege, and throw if invalid
*
diff --git a/users.cpp b/users.cpp
index 8906769..7957482 100644
--- a/users.cpp
+++ b/users.cpp
@@ -94,6 +94,16 @@
return UsersIface::userPrivilege(value);
}
+void Users::setUserPrivilege(const std::string& value)
+{
+ UsersIface::userPrivilege(value);
+}
+
+void Users::setUserGroups(const std::vector<std::string>& groups)
+{
+ UsersIface::userGroups(groups);
+}
+
/** @brief list user privilege
*
*/
diff --git a/users.hpp b/users.hpp
index 77a1079..3541625 100644
--- a/users.hpp
+++ b/users.hpp
@@ -71,6 +71,10 @@
*/
std::string userPrivilege(std::string value) override;
+ void setUserPrivilege(const std::string& value);
+
+ void setUserGroups(const std::vector<std::string>& groups);
+
/** @brief lists user privilege
*
*/