From fdc4667e34cb8ede4529e116b35ed4d411328e08 Mon Sep 17 00:00:00 2001 From: P Dheeraj Srujan Kumar Date: Fri, 11 Feb 2022 05:26:19 +0530 Subject: [PATCH] Add Configure Self support for Event Subscriptions As per DTMF redfish schema privilege registry PATCH and DELETE operations on event subscriptions require ConfigureManager or ConfigureSelf privilege. Currently, only ConfigureManager support was enabled, which implies only Admin user will be able to PATCH and DELETE any given subscription. This commits adds the support to enable ConfigureSelf, which implies, an Operator user will be able to PATCH or DELETE self created subscription. This support is enabled by adding SubscriptionOwner field to the Subscriptions class, so that the Owner of the subscription will be stored when a subscription is created. This Commit also ensures backward compatibility by not mandating the SubscriptionOwner field. Which implies, the older subscriptions which do not have a SubscriptionOwner will not be force removed, but can only be PATCHED or DELETED by Administrator. Tested: - Created 2 Operator level users - Operator1 and Operator2 - Created subscription by POST to /redfish/v1/EventService/Subscriptions using Operator1 - PATCH and DELETE on the subscription failed successfully when using Operator2 user. - PATCH and DELETE was successfull when using Operator1 user. Signed-off-by: P Dheeraj Srujan Kumar --- include/event_service_store.hpp | 11 +++ include/persistent_data.hpp | 1 + .../include/event_service_manager.hpp | 2 + redfish-core/lib/event_service.hpp | 80 ++++++++++++++++--- 4 files changed, 81 insertions(+), 13 deletions(-) diff --git a/include/event_service_store.hpp b/include/event_service_store.hpp index dcc99f1..6997136 100644 --- a/include/event_service_store.hpp +++ b/include/event_service_store.hpp @@ -22,6 +22,7 @@ struct UserSubscription std::vector resourceTypes; boost::beast::http::fields httpHeaders; std::vector metricReportDefinitions; + std::string subscriptionOwner; static std::shared_ptr fromJson(const nlohmann::json& j, const bool loadFromOldConfig = false) @@ -172,6 +173,16 @@ struct UserSubscription subvalue->metricReportDefinitions.emplace_back(*value); } } + else if (element.key() == "SubscriptionOwner") + { + const std::string* value = + element.value().get_ptr(); + if (value == nullptr) + { + continue; + } + subvalue->subscriptionOwner = *value; + } else { BMCWEB_LOG_ERROR diff --git a/include/persistent_data.hpp b/include/persistent_data.hpp index dbd3618..48855ec 100644 --- a/include/persistent_data.hpp +++ b/include/persistent_data.hpp @@ -305,6 +305,7 @@ class ConfigFile {"ResourceTypes", subValue->resourceTypes}, {"SubscriptionType", subValue->subscriptionType}, {"MetricReportDefinitions", subValue->metricReportDefinitions}, + {"SubscriptionOwner", subValue->subscriptionOwner}, }); } persistentFile << data; diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp index 1ba9f21..a1b8921 100644 --- a/redfish-core/include/event_service_manager.hpp +++ b/redfish-core/include/event_service_manager.hpp @@ -692,6 +692,7 @@ class EventServiceManager subValue->resourceTypes = newSub->resourceTypes; subValue->httpHeaders = newSub->httpHeaders; subValue->metricReportDefinitions = newSub->metricReportDefinitions; + subValue->subscriptionOwner = newSub->subscriptionOwner; if (subValue->id.empty()) { @@ -1008,6 +1009,7 @@ class EventServiceManager newSub->resourceTypes = subValue->resourceTypes; newSub->httpHeaders = subValue->httpHeaders; newSub->metricReportDefinitions = subValue->metricReportDefinitions; + newSub->subscriptionOwner = subValue->subscriptionOwner; persistent_data::EventServiceStore::getInstance() .subscriptionsConfigMap.emplace(newSub->id, newSub); diff --git a/redfish-core/lib/event_service.hpp b/redfish-core/lib/event_service.hpp index 9eb845c..2fb2ab1 100644 --- a/redfish-core/lib/event_service.hpp +++ b/redfish-core/lib/event_service.hpp @@ -296,6 +296,7 @@ inline void requestRoutesEventDestinationCollection(App& app) std::make_shared(host, port, path, uriProto); subValue->destinationUrl = destUrl; + subValue->subscriptionOwner = req.session->username; if (subscriptionType) { @@ -577,11 +578,7 @@ inline void requestRoutesEventDestination(App& app) mrdJsonArray; }); BMCWEB_ROUTE(app, "/redfish/v1/EventService/Subscriptions//") - // The below privilege is wrong, it should be ConfigureManager OR - // ConfigureSelf - // https://github.com/openbmc/bmcweb/issues/220 - //.privileges(redfish::privileges::patchEventDestination) - .privileges({{"ConfigureManager"}}) + .privileges(redfish::privileges::patchEventDestination) .methods(boost::beast::http::verb::patch)( [](const crow::Request& req, const std::shared_ptr& asyncResp, @@ -595,6 +592,36 @@ inline void requestRoutesEventDestination(App& app) return; } + Privileges effectiveUserPrivileges = + redfish::getUserPrivileges(req.userRole); + bool isConfigureManager = + effectiveUserPrivileges.isSupersetOf({"ConfigureManager"}); + + if (!isConfigureManager) + { + // If the user does not have Configure manager privilege + // then the user must be an Operator (i.e. Configure + // Components and Self) + // We need to ensure that the User is the actual owner of the + // Subscription being patched + // This also supports backward compatibility as subscription + // owner would be empty which would not be equal to current + // user, enabling only Admin to be able to patch the + // Subscription + + if (subValue->subscriptionOwner == "") + { + messages::insufficientPrivilege(asyncResp->res); + return; + } + + if (subValue->subscriptionOwner != req.session->username) + { + messages::insufficientPrivilege(asyncResp->res); + return; + } + } + std::optional context; std::optional retryPolicy; std::optional> headers; @@ -653,22 +680,49 @@ inline void requestRoutesEventDestination(App& app) EventServiceManager::getInstance().updateSubscription(param); }); BMCWEB_ROUTE(app, "/redfish/v1/EventService/Subscriptions//") - // The below privilege is wrong, it should be ConfigureManager OR - // ConfigureSelf - // https://github.com/openbmc/bmcweb/issues/220 - //.privileges(redfish::privileges::deleteEventDestination) - .privileges({{"ConfigureManager"}}) + .privileges(redfish::privileges::deleteEventDestination) .methods(boost::beast::http::verb::delete_)( - [](const crow::Request&, + [](const crow::Request& req, const std::shared_ptr& asyncResp, const std::string& param) { - if (!EventServiceManager::getInstance().isSubscriptionExist( - param)) + std::shared_ptr subValue = + EventServiceManager::getInstance().getSubscription(param); + if (subValue == nullptr) { asyncResp->res.result( boost::beast::http::status::not_found); return; } + + Privileges effectiveUserPrivileges = + redfish::getUserPrivileges(req.userRole); + bool isConfigureManager = + effectiveUserPrivileges.isSupersetOf({"ConfigureManager"}); + + if (!isConfigureManager) + { + // If the user does not have Configure manager privilege + // then the user must be an Operator (i.e. Configure + // Components and Self) + // We need to ensure that the User is the actual owner of the + // Subscription being deleted + // This also supports backward compatibility as subscription + // owner would be empty which would not be equal to current + // user, enabling only Admin to be able to patch the + // Subscription + + if (subValue->subscriptionOwner == "") + { + messages::insufficientPrivilege(asyncResp->res); + return; + } + + if (subValue->subscriptionOwner != req.session->username) + { + messages::insufficientPrivilege(asyncResp->res); + return; + } + } EventServiceManager::getInstance().deleteSubscription(param); }); } -- 2.17.1