From dc511aa73001a593a16dbcdaa5d53f320e4c7818 Mon Sep 17 00:00:00 2001
From: Ed Tanous <ed@tanous.net>
Date: Wed, 21 Oct 2020 12:33:42 -0700
Subject: Timeout is not per-session

fix regression on 5fb91ba400e0482813cf5e1a86fdca17468d0a6a.

Timeout is a global setting, not a per-session setting.  This caused
problems with regenerating it, as session restoration doesn't follow the
"best effort" policy we've done before.

This commit:
1. Makes Session::fromJson more robust against extra keys.
2. Disallowed reading in client_id if IBM_Management_console isn't
enabled.
3. Moves timeout to the proper place in the persistent config file.

Resolves https://github.com/openbmc/bmcweb/issues/158

Tested:
Downloaded to bmc, cleared bmcweb_persistent_data.json, then logged in
using webui-vue.
Rebooted BMC.
Reloaded /redfish/v1/SessionService/Sessions/<sessionid> and observed
that all data restored properly.  Unclear why, but ClientOriginIPAddress
seems broken, but that seems true prior to this patch.

Data that got returned is included for completeness.
{
"@odata.id": "/redfish/v1/SessionService/Sessions/BKqK5dNfNS",
"@odata.type": "#Session.v1_3_0.Session",
"ClientOriginIPAddress": "",
"Description": "Manager User Session",
"Id": "BKqK5dNfNS",
"Name": "User Session",
"UserName": "root"
}

Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I716431fd4775af63715d07973f723caa8cb34259
---
 include/sessions.hpp | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

(limited to 'include/sessions.hpp')

diff --git a/include/sessions.hpp b/include/sessions.hpp
index 88fd487024..dc6ac1f4fe 100644
--- a/include/sessions.hpp
+++ b/include/sessions.hpp
@@ -79,7 +79,7 @@ struct UserSession
             {
                 BMCWEB_LOG_ERROR << "Error reading persistent store.  Property "
                                  << element.key() << " was not of type string";
-                return nullptr;
+                continue;
             }
             if (element.key() == "unique_id")
             {
@@ -97,10 +97,12 @@ struct UserSession
             {
                 userSession->username = *thisValue;
             }
+#ifdef BMCWEB_ENABLE_IBM_MANAGEMENT_CONSOLE
             else if (element.key() == "client_id")
             {
                 userSession->clientId = *thisValue;
             }
+#endif
             else if (element.key() == "client_ip")
             {
                 userSession->clientIp = *thisValue;
@@ -111,9 +113,20 @@ struct UserSession
                 BMCWEB_LOG_ERROR
                     << "Got unexpected property reading persistent file: "
                     << element.key();
-                return nullptr;
+                continue;
             }
         }
+        // If any of these fields are missing, we can't restore the session, as
+        // we don't have enough information.  These 4 fields have been present
+        // in every version of this file in bmcwebs history, so any file, even
+        // on upgrade, should have these present
+        if (userSession->uniqueId.empty() || userSession->username.empty() ||
+            userSession->sessionToken.empty() || userSession->csrfToken.empty())
+        {
+            BMCWEB_LOG_DEBUG << "Session missing required security "
+                                "information, refusing to restore";
+            return nullptr;
+        }
 
         // For now, sessions that were persisted through a reboot get their idle
         // timer reset.  This could probably be overcome with a better
-- 
cgit v1.2.3