Clean up Account Service to use readJson

Use multiple level direct read.

Tested: Visual only. Need help if anyone wants to test.

Change-Id: I67c77bdd42a05a42f9cd1b40dc74517dceebdaad
Signed-off-by: Ed Tanous <ed@tanous.net>

1 files changed, 78 insertions, 151 deletions

diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index edf3cf7ef2..3a1869f095 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -360,13 +360,16 @@ inline void parseLDAPConfigData(nlohmann::json& jsonResponse,
 inline void handleRoleMapPatch(
     const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
     const std::vector<std::pair<std::string, LDAPRoleMapData>>& roleMapObjData,
-    const std::string& serverType, const std::vector<nlohmann::json>& input)
+    const std::string& serverType,
+    std::vector<std::variant<nlohmann::json::object_t, std::nullptr_t>>& input)
 {
     for (size_t index = 0; index < input.size(); index++)
     {
-        const nlohmann::json& thisJson = input[index];
-
-        if (thisJson.is_null())
+        std::variant<nlohmann::json::object_t, std::nullptr_t>& thisJson =
+            input[index];
+        nlohmann::json::object_t* obj =
+            std::get_if<nlohmann::json::object_t>(&thisJson);
+        if (obj == nullptr)
         {
             // delete the existing object
             if (index < roleMapObjData.size())
@@ -389,13 +392,13 @@ inline void handleRoleMapPatch(
             else
             {
                 BMCWEB_LOG_ERROR("Can't delete the object");
-                messages::propertyValueTypeError(asyncResp->res, thisJson,
+                messages::propertyValueTypeError(asyncResp->res, "null",
                                                  "RemoteRoleMapping/" +
                                                      std::to_string(index));
                 return;
             }
         }
-        else if (thisJson.empty())
+        else if (obj->empty())
         {
             // Don't do anything for the empty objects,parse next json
             // eg {"RemoteRoleMapping",[{}]}
@@ -406,12 +409,8 @@ inline void handleRoleMapPatch(
             std::optional<std::string> remoteGroup;
             std::optional<std::string> localRole;
 
-            // This is a copy, but it's required in this case because of how
-            // readJson is structured
-            nlohmann::json thisJsonCopy = thisJson;
-            if (!json_util::readJson(thisJsonCopy, asyncResp->res,
-                                     "RemoteGroup", remoteGroup, "LocalRole",
-                                     localRole))
+            if (!json_util::readJsonObject(*obj, asyncResp->res, "RemoteGroup",
+                                           remoteGroup, "LocalRole", localRole))
             {
                 continue;
             }
@@ -514,8 +513,9 @@ inline void getLDAPConfigData(const std::string& ldapType,
 
     dbus::utility::getDbusObject(
         ldapConfigObjectName, interfaces,
-        [callback, ldapType](const boost::system::error_code& ec,
-                             const dbus::utility::MapperGetObject& resp) {
+        [callback,
+         ldapType](const boost::system::error_code& ec,
+                   const dbus::utility::MapperGetObject& resp) mutable {
         if (ec || resp.empty())
         {
             BMCWEB_LOG_WARNING(
@@ -528,9 +528,9 @@ inline void getLDAPConfigData(const std::string& ldapType,
         sdbusplus::message::object_path path(ldapRootObject);
         dbus::utility::getManagedObjects(
             service, path,
-            [callback,
-             ldapType](const boost::system::error_code& ec2,
-                       const dbus::utility::ManagedObjectType& ldapObjects) {
+            [callback, ldapType](
+                const boost::system::error_code& ec2,
+                const dbus::utility::ManagedObjectType& ldapObjects) mutable {
             LDAPConfigData confData{};
             if (ec2)
             {
@@ -669,71 +669,6 @@ inline void getLDAPConfigData(const std::string& ldapType,
 }
 
 /**
- * @brief parses the authentication section under the LDAP
- * @param input JSON data
- * @param asyncResp pointer to the JSON response
- * @param userName  userName to be filled from the given JSON.
- * @param password  password to be filled from the given JSON.
- */
-inline void parseLDAPAuthenticationJson(
-    nlohmann::json input, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
-    std::optional<std::string>& username, std::optional<std::string>& password)
-{
-    std::optional<std::string> authType;
-
-    if (!json_util::readJson(input, asyncResp->res, "AuthenticationType",
-                             authType, "Username", username, "Password",
-                             password))
-    {
-        return;
-    }
-    if (!authType)
-    {
-        return;
-    }
-    if (*authType != "UsernameAndPassword")
-    {
-        messages::propertyValueNotInList(asyncResp->res, *authType,
-                                         "AuthenticationType");
-        return;
-    }
-}
-/**
- * @brief parses the LDAPService section under the LDAP
- * @param input JSON data
- * @param asyncResp pointer to the JSON response
- * @param baseDNList baseDN to be filled from the given JSON.
- * @param userNameAttribute  userName to be filled from the given JSON.
- * @param groupaAttribute  password to be filled from the given JSON.
- */
-
-inline void
-    parseLDAPServiceJson(nlohmann::json input,
-                         const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
-                         std::optional<std::vector<std::string>>& baseDNList,
-                         std::optional<std::string>& userNameAttribute,
-                         std::optional<std::string>& groupsAttribute)
-{
-    std::optional<nlohmann::json> searchSettings;
-
-    if (!json_util::readJson(input, asyncResp->res, "SearchSettings",
-                             searchSettings))
-    {
-        return;
-    }
-    if (!searchSettings)
-    {
-        return;
-    }
-    if (!json_util::readJson(*searchSettings, asyncResp->res,
-                             "BaseDistinguishedNames", baseDNList,
-                             "UsernameAttribute", userNameAttribute,
-                             "GroupsAttribute", groupsAttribute))
-    {
-        return;
-    }
-}
-/**
  * @brief updates the LDAP server address and updates the
           json response with the new value.
  * @param serviceAddressList address to be updated.
@@ -876,29 +811,23 @@ inline void handleServiceEnablePatch(
                     ldapServerElementName + "/ServiceEnabled", serviceEnabled);
 }
 
-inline void
-    handleAuthMethodsPatch(nlohmann::json& input,
-                           const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
+struct AuthMethods
 {
     std::optional<bool> basicAuth;
     std::optional<bool> cookie;
     std::optional<bool> sessionToken;
     std::optional<bool> xToken;
     std::optional<bool> tls;
+};
 
-    if (!json_util::readJson(input, asyncResp->res, "BasicAuth", basicAuth,
-                             "Cookie", cookie, "SessionToken", sessionToken,
-                             "XToken", xToken, "TLS", tls))
-    {
-        BMCWEB_LOG_ERROR("Cannot read values from AuthMethod tag");
-        return;
-    }
-
-    // Make a copy of methods configuration
-    persistent_data::AuthConfigMethods authMethodsConfig =
+inline void
+    handleAuthMethodsPatch(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+                           const AuthMethods& auth)
+{
+    persistent_data::AuthConfigMethods& authMethodsConfig =
         persistent_data::SessionStore::getInstance().getAuthMethodsConfig();
 
-    if (basicAuth)
+    if (auth.basicAuth)
     {
 #ifndef BMCWEB_ENABLE_BASIC_AUTHENTICATION
         messages::actionNotSupported(
@@ -906,10 +835,10 @@ inline void
             "Setting BasicAuth when basic-auth feature is disabled");
         return;
 #endif
-        authMethodsConfig.basic = *basicAuth;
+        authMethodsConfig.basic = *auth.basicAuth;
     }
 
-    if (cookie)
+    if (auth.cookie)
     {
 #ifndef BMCWEB_ENABLE_COOKIE_AUTHENTICATION
         messages::actionNotSupported(
@@ -917,10 +846,10 @@ inline void
             "Setting Cookie when cookie-auth feature is disabled");
         return;
 #endif
-        authMethodsConfig.cookie = *cookie;
+        authMethodsConfig.cookie = *auth.cookie;
     }
 
-    if (sessionToken)
+    if (auth.sessionToken)
     {
 #ifndef BMCWEB_ENABLE_SESSION_AUTHENTICATION
         messages::actionNotSupported(
@@ -928,10 +857,10 @@ inline void
             "Setting SessionToken when session-auth feature is disabled");
         return;
 #endif
-        authMethodsConfig.sessionToken = *sessionToken;
+        authMethodsConfig.sessionToken = *auth.sessionToken;
     }
 
-    if (xToken)
+    if (auth.xToken)
     {
 #ifndef BMCWEB_ENABLE_XTOKEN_AUTHENTICATION
         messages::actionNotSupported(
@@ -939,10 +868,10 @@ inline void
             "Setting XToken when xtoken-auth feature is disabled");
         return;
 #endif
-        authMethodsConfig.xtoken = *xToken;
+        authMethodsConfig.xtoken = *auth.xToken;
     }
 
-    if (tls)
+    if (auth.tls)
     {
 #ifndef BMCWEB_ENABLE_MUTUAL_TLS_AUTHENTICATION
         messages::actionNotSupported(
@@ -950,7 +879,7 @@ inline void
             "Setting TLS when mutual-tls-auth feature is disabled");
         return;
 #endif
-        authMethodsConfig.tls = *tls;
+        authMethodsConfig.tls = *auth.tls;
     }
 
     if (!authMethodsConfig.basic && !authMethodsConfig.cookie &&
@@ -979,7 +908,7 @@ inline void
  * @param serverType Type of LDAP server(openLDAP/ActiveDirectory)
  */
 
-inline void handleLDAPPatch(nlohmann::json& input,
+inline void handleLDAPPatch(nlohmann::json::object_t& input,
                             const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
                             const std::string& serverType)
 {
@@ -997,8 +926,7 @@ inline void handleLDAPPatch(nlohmann::json& input,
         return;
     }
 
-    std::optional<nlohmann::json> authentication;
-    std::optional<nlohmann::json> ldapService;
+    std::optional<std::string> authType;
     std::optional<std::vector<std::string>> serviceAddressList;
     std::optional<bool> serviceEnabled;
     std::optional<std::vector<std::string>> baseDNList;
@@ -1006,27 +934,32 @@ inline void handleLDAPPatch(nlohmann::json& input,
     std::optional<std::string> groupsAttribute;
     std::optional<std::string> userName;
     std::optional<std::string> password;
-    std::optional<std::vector<nlohmann::json>> remoteRoleMapData;
-
-    if (!json_util::readJson(input, asyncResp->res, "Authentication",
-                             authentication, "LDAPService", ldapService,
-                             "ServiceAddresses", serviceAddressList,
-                             "ServiceEnabled", serviceEnabled,
-                             "RemoteRoleMapping", remoteRoleMapData))
+    std::optional<
+        std::vector<std::variant<nlohmann::json::object_t, std::nullptr_t>>>
+        remoteRoleMapData;
+    // clang-format off
+    if (!json_util::readJsonObject(input, asyncResp->res,
+          "Authentication/AuthenticationType", authType,
+          "Authentication/Username", userName,
+          "Authentication/Password", password,
+          "LDAPService/SearchSettings/BaseDistinguishedNames", baseDNList,
+          "LDAPService/SearchSettings/UsernameAttribute", userNameAttribute,
+          "LDAPService/SearchSettings/GroupsAttribute", groupsAttribute,
+          "ServiceAddresses", serviceAddressList,
+          "ServiceEnabled", serviceEnabled,
+          "RemoteRoleMapping", remoteRoleMapData))
     {
         return;
     }
+    // clang-format on
 
-    if (authentication)
-    {
-        parseLDAPAuthenticationJson(*authentication, asyncResp, userName,
-                                    password);
-    }
-    if (ldapService)
+    if (authType && *authType != "UsernameAndPassword")
     {
-        parseLDAPServiceJson(*ldapService, asyncResp, baseDNList,
-                             userNameAttribute, groupsAttribute);
+        messages::propertyValueNotInList(asyncResp->res, *authType,
+                                         "AuthenticationType");
+        return;
     }
+
     if (serviceAddressList)
     {
         if (serviceAddressList->empty())
@@ -1061,7 +994,7 @@ inline void handleLDAPPatch(nlohmann::json& input,
         [asyncResp, userName, password, baseDNList, userNameAttribute,
          groupsAttribute, serviceAddressList, serviceEnabled, dbusObjectPath,
          remoteRoleMapData](bool success, const LDAPConfigData& confData,
-                            const std::string& serverT) {
+                            const std::string& serverT) mutable {
         if (!success)
         {
             messages::internalError(asyncResp->res);
@@ -1256,13 +1189,13 @@ inline void
         return;
     }
 
+    const persistent_data::AuthConfigMethods& authMethodsConfig =
+        persistent_data::SessionStore::getInstance().getAuthMethodsConfig();
+
     asyncResp->res.addHeader(
         boost::beast::http::field::link,
         "</redfish/v1/JsonSchemas/AccountService/AccountService.json>; rel=describedby");
 
-    const persistent_data::AuthConfigMethods& authMethodsConfig =
-        persistent_data::SessionStore::getInstance().getAuthMethodsConfig();
-
     nlohmann::json& json = asyncResp->res.jsonValue;
     json["@odata.id"] = "/redfish/v1/AccountService";
     json["@odata.type"] = "#AccountService."
@@ -1371,19 +1304,27 @@ inline void handleAccountServicePatch(
     std::optional<uint16_t> lockoutThreshold;
     std::optional<uint8_t> minPasswordLength;
     std::optional<uint16_t> maxPasswordLength;
-    std::optional<nlohmann::json> ldapObject;
-    std::optional<nlohmann::json> activeDirectoryObject;
-    std::optional<nlohmann::json> oemObject;
-
+    std::optional<nlohmann::json::object_t> ldapObject;
+    std::optional<nlohmann::json::object_t> activeDirectoryObject;
+    AuthMethods auth;
+    // clang-format off
     if (!json_util::readJsonPatch(
-            req, asyncResp->res, "AccountLockoutDuration", unlockTimeout,
-            "AccountLockoutThreshold", lockoutThreshold, "MaxPasswordLength",
-            maxPasswordLength, "MinPasswordLength", minPasswordLength, "LDAP",
-            ldapObject, "ActiveDirectory", activeDirectoryObject, "Oem",
-            oemObject))
+            req, asyncResp->res,
+            "AccountLockoutDuration", unlockTimeout,
+            "AccountLockoutThreshold", lockoutThreshold,
+            "MaxPasswordLength", maxPasswordLength,
+            "MinPasswordLength", minPasswordLength,
+            "LDAP", ldapObject,
+            "ActiveDirectory", activeDirectoryObject,
+            "Oem/OpenBMC/AuthMethods/BasicAuth", auth.basicAuth,
+            "Oem/OpenBMC/AuthMethods/Cookie", auth.cookie,
+            "Oem/OpenBMC/AuthMethods/SessionToken", auth.sessionToken,
+            "Oem/OpenBMC/AuthMethods/XToken", auth.xToken,
+            "Oem/OpenBMC/AuthMethods/TLS", auth.tls))
     {
         return;
     }
+    // clang-format on
 
     if (minPasswordLength)
     {
@@ -1404,21 +1345,7 @@ inline void handleAccountServicePatch(
         handleLDAPPatch(*ldapObject, asyncResp, "LDAP");
     }
 
-    if (std::optional<nlohmann::json> oemOpenBMCObject;
-        oemObject && json_util::readJson(*oemObject, asyncResp->res, "OpenBMC",
-                                         oemOpenBMCObject))
-    {
-        if (std::optional<nlohmann::json> authMethodsObject;
-            oemOpenBMCObject &&
-            json_util::readJson(*oemOpenBMCObject, asyncResp->res,
-                                "AuthMethods", authMethodsObject))
-        {
-            if (authMethodsObject)
-            {
-                handleAuthMethodsPatch(*authMethodsObject, asyncResp);
-            }
-        }
-    }
+    handleAuthMethodsPatch(asyncResp, auth);
 
     if (activeDirectoryObject)
     {