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>
diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index 7028260..9f98991 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -1323,8 +1323,14 @@
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 @@
{"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 @@
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 @@
{
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");