Update ManagerAccountCollection Privilege

For the ManagerAccountCollection resource,
/redfish/v1/AccountService/Accounts/, allow a "ConfigureSelf"
user to GET but only return the user's account if the user does
not have ConfigureUsers privilege.

Took this code from other places in account_service.
https://github.com/openbmc/bmcweb/blob/61dbeef97168db1a1f7a351c5f95e09afd361e48/redfish-core/lib/account_service.hpp#L1528

There was some question whether all accounts should be returned,
Redfish clarified that only the user's account should be returned
without ConfigureUsers privilege.

https://redfishforum.com/thread/281/manageraccountcollection-change-allows-account-enumeration
"we assumed that the Login privilege would only pertain to the
current account and not allow viewing of other accounts"

This fixes 2 Redfish validator errors if running the validator
as a Readonly or Operator role.

"ERROR - Accounts: GET of resource at URI
/redfish/v1/AccountService/Accounts returned HTTP 403. Check URI."

"ERROR - /redfish/v1/AccountService/Accounts:  URI could not be
acquired: 403"

This was changed in Redfish 2019.3, redfish issue 1914 explains
more.

Tested: Ran the validator as operator role and admin role.
        No errors.
As root:
 curl -k https://${bmc}/redfish/v1/AccountService/Accounts/
{
  "@odata.id": "/redfish/v1/AccountService/Accounts",
  "@odata.type": "#ManagerAccountCollection.ManagerAccountCollection",
  "Description": "BMC User Accounts",
  "Members": [
    {
      "@odata.id": "/redfish/v1/AccountService/Accounts/readonly"
    },
    {
      "@odata.id": "/redfish/v1/AccountService/Accounts/operator"
    },
    {
      "@odata.id": "/redfish/v1/AccountService/Accounts/JimHalpert"
    },
    {
      "@odata.id": "/redfish/v1/AccountService/Accounts/root"
    }
  ],
  "Members@odata.count": 4,

As Operator:
curl -k https://${bmc}/redfish/v1/AccountService/Accounts/
{
  "@odata.id": "/redfish/v1/AccountService/Accounts",
  "@odata.type": "#ManagerAccountCollection.ManagerAccountCollection",
  "Description": "BMC User Accounts",
  "Members": [
    {
      "@odata.id": "/redfish/v1/AccountService/Accounts/operator"
    }
  ],
  "Members@odata.count": 1,

Change-Id: I0694011ed3c4ecd3ea0c386fc24d086be39ac804
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

1 files changed, 24 insertions, 7 deletions

diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index 7028260327..9f989911c9 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -1323,8 +1323,14 @@ class AccountsCollection : public Node
         Node(app, "/redfish/v1/AccountService/Accounts/")
     {
         entityPrivileges = {
+            // According to the PrivilegeRegistry, GET should actually be
+            // "Login". A "Login" only privilege would return an empty "Members"
+            // list. Not going to worry about this since none of the defined
+            // roles are just "Login". E.g. Readonly is {"Login",
+            // "ConfigureSelf"}. In the rare event anyone defines a role that
+            // has Login but not ConfigureSelf, implement this.
             {boost::beast::http::verb::get,
-             {{"ConfigureUsers"}, {"ConfigureManager"}}},
+             {{"ConfigureUsers"}, {"ConfigureSelf"}}},
             {boost::beast::http::verb::head, {{"Login"}}},
             {boost::beast::http::verb::patch, {{"ConfigureUsers"}}},
             {boost::beast::http::verb::put, {{"ConfigureUsers"}}},
@@ -1344,8 +1350,8 @@ class AccountsCollection : public Node
                          {"Description", "BMC User Accounts"}};
 
         crow::connections::systemBus->async_method_call(
-            [asyncResp](const boost::system::error_code ec,
-                        const ManagedObjectType& users) {
+            [asyncResp, &req, this](const boost::system::error_code ec,
+                                    const ManagedObjectType& users) {
                 if (ec)
                 {
                     messages::internalError(asyncResp->res);
@@ -1356,7 +1362,6 @@ class AccountsCollection : public Node
                     asyncResp->res.jsonValue["Members"];
                 memberArray = nlohmann::json::array();
 
-                asyncResp->res.jsonValue["Members@odata.count"] = users.size();
                 for (auto& user : users)
                 {
                     const std::string& path =
@@ -1370,10 +1375,22 @@ class AccountsCollection : public Node
                     {
                         lastIndex += 1;
                     }
-                    memberArray.push_back(
-                        {{"@odata.id", "/redfish/v1/AccountService/Accounts/" +
-                                           path.substr(lastIndex)}});
+
+                    // As clarified by Redfish here:
+                    // https://redfishforum.com/thread/281/manageraccountcollection-change-allows-account-enumeration
+                    // Users without ConfigureUsers, only see their own account.
+                    // Users with ConfigureUsers, see all accounts.
+                    if (req.session->username == path.substr(lastIndex) ||
+                        isAllowedWithoutConfigureSelf(req))
+                    {
+                        memberArray.push_back(
+                            {{"@odata.id",
+                              "/redfish/v1/AccountService/Accounts/" +
+                                  path.substr(lastIndex)}});
+                    }
                 }
+                asyncResp->res.jsonValue["Members@odata.count"] =
+                    memberArray.size();
             },
             "xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user",
             "org.freedesktop.DBus.ObjectManager", "GetManagedObjects");