account_service: redfish user Patch error handling
Modified doPatch method to populate redfish user update error codes.
Tested:
Tested user updates with below scenarios
1)Provided username is not exist
2)Replace username already user exists
3)Replace Username is NULL/Invalid
4)Replace username is not starting with alphabet
5)Replace username exceeds more than 16 characters
6)Password is not valid for Replace/existing username
Redfish validator test results:
1 failProp errors in /redfish/v1/Systems/system/LogServices/EventLog
1 problemResource errors in /redfish/v1/Systems/system/LogServices/
EventLog/Entries
Counter({'skipOptional': 17887, 'pass': 12133, 'passGet': 1285,
'metadataNamespaces': 1047, 'serviceNamespaces': 69, 'reflink': 9,
'passAction': 7, 'warningPresent': 6, 'optionalAction': 6,
'repeat': 3, 'invalidPropertyValue': 3, 'failErrorPresent': 1,
'err.LogEntryCollection.LogEntryCollection': 1, 'failProp': 1,
'unvalidated': 1, 'problemResource': 1,
'unverifiedComplexAdditional': 1, 'warnTrailingSlashLink': 1})
Validation has failed: 3 problems found
Signed-off-by: jayaprakash Mutyala <mutyalax.jayaprakash@intel.com>
Change-Id: Ibee448c5d5c4f38c5c4cacda757864593f6001fc
diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index 07efeb5..59e2d1c 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -113,6 +113,54 @@
return "";
}
+void userErrorMessageHandler(const sd_bus_error* e,
+ std::shared_ptr<AsyncResp> asyncResp,
+ const std::string& newUser,
+ const std::string& username)
+{
+ const char* errorMessage = e->name;
+ if (e == nullptr)
+ {
+ messages::internalError(asyncResp->res);
+ return;
+ }
+
+ if (strcmp(errorMessage,
+ "xyz.openbmc_project.User.Common.Error.UserNameExists") == 0)
+ {
+ messages::resourceAlreadyExists(asyncResp->res,
+ "#ManagerAccount.v1_0_3.ManagerAccount",
+ "UserName", newUser);
+ }
+ else if (strcmp(errorMessage, "xyz.openbmc_project.User.Common.Error."
+ "UserNameDoesNotExist") == 0)
+ {
+ messages::resourceNotFound(
+ asyncResp->res, "#ManagerAccount.v1_0_3.ManagerAccount", username);
+ }
+ else if (strcmp(errorMessage,
+ "xyz.openbmc_project.Common.Error.InvalidArgument") == 0)
+ {
+ messages::propertyValueFormatError(asyncResp->res, newUser, "UserName");
+ }
+ else if (strcmp(errorMessage,
+ "xyz.openbmc_project.User.Common.Error.NoResource") == 0)
+ {
+ messages::createLimitReachedForResource(asyncResp->res);
+ }
+ else if (strcmp(errorMessage, "xyz.openbmc_project.User.Common.Error."
+ "UserNameGroupFail") == 0)
+ {
+ messages::propertyValueFormatError(asyncResp->res, newUser, "UserName");
+ }
+ else
+ {
+ messages::internalError(asyncResp->res);
+ }
+
+ return;
+}
+
void parseLDAPConfigData(nlohmann::json& json_response,
const LDAPConfigData& confData,
const std::string& ldapType)
@@ -1298,7 +1346,8 @@
return;
}
- if (!pamUpdatePassword(username, password))
+ if (pamUpdatePassword(username, password) !=
+ PAM_SUCCESS)
{
// At this point we have a user that's been created,
// but the password set failed.Something is wrong,
@@ -1505,10 +1554,12 @@
const std::string& username = params[0];
- if (!newUserName)
+ // if user name is not provided in the patch method or if it
+ // matches the user name in the URI, then we are treating it as updating
+ // user properties other then username. If username provided doesn't
+ // match the URI, then we are treating this as user rename request.
+ if (!newUserName || (newUserName.value() == username))
{
- // If the username isn't being updated, we can update the
- // properties directly
updateUserProperties(asyncResp, username, password, enabled, roleId,
locked);
return;
@@ -1518,14 +1569,13 @@
crow::connections::systemBus->async_method_call(
[this, asyncResp, username, password(std::move(password)),
roleId(std::move(roleId)), enabled(std::move(enabled)),
- newUser{*newUserName}, locked(std::move(locked))](
- const boost::system::error_code ec) {
+ newUser{std::string(*newUserName)},
+ locked(std::move(locked))](const boost::system::error_code ec,
+ sdbusplus::message::message& m) {
if (ec)
{
- BMCWEB_LOG_ERROR << "D-Bus responses error: " << ec;
- messages::resourceNotFound(
- asyncResp->res,
- "#ManagerAccount.v1_0_3.ManagerAccount", username);
+ userErrorMessageHandler(m.get_error(), asyncResp,
+ newUser, username);
return;
}
@@ -1545,16 +1595,6 @@
std::optional<std::string> roleId,
std::optional<bool> locked)
{
- if (password)
- {
- if (!pamUpdatePassword(username, *password))
- {
- BMCWEB_LOG_ERROR << "pamUpdatePassword Failed";
- messages::internalError(asyncResp->res);
- return;
- }
- }
-
std::string dbusObjectPath = "/xyz/openbmc_project/user/" + username;
dbus::utility::escapePathForDbus(dbusObjectPath);
@@ -1566,9 +1606,36 @@
asyncResp{std::move(asyncResp)}](int rc) {
if (!rc)
{
- messages::invalidObject(asyncResp->res, username.c_str());
+ messages::resourceNotFound(
+ asyncResp->res, "#ManagerAccount.v1_0_3.ManagerAccount",
+ username);
return;
}
+
+ if (password)
+ {
+ int retval = pamUpdatePassword(username, *password);
+
+ if (retval == PAM_USER_UNKNOWN)
+ {
+ messages::resourceNotFound(
+ asyncResp->res,
+ "#ManagerAccount.v1_0_3.ManagerAccount", username);
+ }
+ else if (retval == PAM_AUTHTOK_ERR)
+ {
+ // If password is invalid
+ messages::propertyValueFormatError(
+ asyncResp->res, *password, "Password");
+ BMCWEB_LOG_ERROR << "pamUpdatePassword Failed";
+ }
+ else if (retval != PAM_SUCCESS)
+ {
+ messages::internalError(asyncResp->res);
+ return;
+ }
+ }
+
if (enabled)
{
crow::connections::systemBus->async_method_call(