summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEd Tanous <edtanous@google.com>2022-07-08 06:44:54 +0300
committerEd Tanous <ed@tanous.net>2022-12-07 01:17:31 +0300
commit2d6cb56b6b47c3fbb0d234ade5c1208edb69ef1f (patch)
tree62f0c97e3eb291d6b0bf943fa6868bb8624ae056
parentc0bdf22334f859d1d2a522d3841160f579a3cbab (diff)
downloadbmcweb-2d6cb56b6b47c3fbb0d234ade5c1208edb69ef1f.tar.xz
Implement If-Match header in Http layer
If-Match is a header in the HTTP specification[1] designed for handling atomic operations within a given HTTP tree. It allows a mechanism for an implementation to explicitly declare "only take this action if the resource has not been changed". While most things within the Redfish tree don't require this level of interlocking, it continues to round out our redfish support for the specific use cases that might require it. Redfish specification 6.5 states: If a service supports the return of the ETag header on a resource, the service may respond with HTTP 428 status code if the If-Match or If-None-Match header is missing from the PUT or PATCH request for the same resource, as specified in RFC6585 This commit implements that behavior for all handlers to follow the following flow. If If-Match is present Repeat the same request as a GET Compare the ETag produced by the GET, to the one provided by If-Match If they don't match, return 428 if they do match, re-run the query. [1] https://www.rfc-editor.org/rfc/rfc2616#section-14.24 As a consequence, this requires declaring copy and move constructors onto the Request object, so the request object can have its lifetime extended through a request, which is very uncommon. Tested: Tests run on /redfish/v1/AccountService/Accounts/root PATCH with correct If-Match returns 200 success PATCH with an incorrect If-Match returns 419 precondition required GET returns the resource as expected Redfish service validator passes Redfish protocol validator passes! ! ! ! ! Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I530ab255259c32fe4402eb8e5104bd091925c77b
-rw-r--r--http/http_request.hpp20
-rw-r--r--http/http_response.hpp28
-rw-r--r--http/routing.hpp10
-rw-r--r--redfish-core/include/query.hpp74
4 files changed, 116 insertions, 16 deletions
diff --git a/http/http_request.hpp b/http/http_request.hpp
index cb7b71bfa2..deaba31c41 100644
--- a/http/http_request.hpp
+++ b/http/http_request.hpp
@@ -45,8 +45,24 @@ struct Request
}
}
- Request(const Request&) = delete;
- Request(const Request&&) = delete;
+ Request(const Request& other) :
+ req(other.req), fields(req.base()), isSecure(other.isSecure),
+ body(req.body()), ioService(other.ioService),
+ ipAddress(other.ipAddress), session(other.session),
+ userRole(other.userRole)
+ {
+ setUrlInfo();
+ }
+
+ Request(Request&& other) noexcept :
+ req(std::move(other.req)), fields(req.base()), isSecure(other.isSecure),
+ body(req.body()), ioService(other.ioService),
+ ipAddress(std::move(other.ipAddress)),
+ session(std::move(other.session)), userRole(std::move(other.userRole))
+ {
+ setUrlInfo();
+ }
+
Request& operator=(const Request&) = delete;
Request& operator=(const Request&&) = delete;
~Request() = default;
diff --git a/http/http_response.hpp b/http/http_response.hpp
index 213117dbd6..c1f25a543c 100644
--- a/http/http_response.hpp
+++ b/http/http_response.hpp
@@ -161,18 +161,28 @@ struct Response
stringResponse->body() += std::string(bodyPart);
}
- void end()
+ std::string computeEtag() const
{
// Only set etag if this request succeeded
- if (result() == boost::beast::http::status::ok)
+ if (result() != boost::beast::http::status::ok)
+ {
+ return "";
+ }
+ // and the json response isn't empty
+ if (jsonValue.empty())
+ {
+ return "";
+ }
+ size_t hashval = std::hash<nlohmann::json>{}(jsonValue);
+ return "\"" + intToHexString(hashval, 8) + "\"";
+ }
+
+ void end()
+ {
+ std::string etag = computeEtag();
+ if (!etag.empty())
{
- // and the json response isn't empty
- if (!jsonValue.empty())
- {
- size_t hashval = std::hash<nlohmann::json>{}(jsonValue);
- std::string hexVal = "\"" + intToHexString(hashval, 8) + "\"";
- addHeader(boost::beast::http::field::etag, hexVal);
- }
+ addHeader(boost::beast::http::field::etag, etag);
}
if (completed)
{
diff --git a/http/routing.hpp b/http/routing.hpp
index 650aeb5aed..ed1b7e489b 100644
--- a/http/routing.hpp
+++ b/http/routing.hpp
@@ -1370,11 +1370,12 @@ class Router
rule.handle(req, asyncResp, params);
return;
}
+ std::string username = req.session->username;
crow::connections::systemBus->async_method_call(
- [&req, asyncResp, &rule,
- params](const boost::system::error_code ec,
- const dbus::utility::DBusPropertiesMap& userInfoMap) {
+ [req{std::move(req)}, asyncResp, &rule, params](
+ const boost::system::error_code ec,
+ const dbus::utility::DBusPropertiesMap& userInfoMap) mutable {
if (ec)
{
BMCWEB_LOG_ERROR << "GetUserInfo failed...";
@@ -1473,8 +1474,7 @@ class Router
rule.handle(req, asyncResp, params);
},
"xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user",
- "xyz.openbmc_project.User.Manager", "GetUserInfo",
- req.session->username);
+ "xyz.openbmc_project.User.Manager", "GetUserInfo", username);
}
void debugPrint()
diff --git a/redfish-core/include/query.hpp b/redfish-core/include/query.hpp
index 007b262244..f9386dae18 100644
--- a/redfish-core/include/query.hpp
+++ b/redfish-core/include/query.hpp
@@ -31,6 +31,75 @@
namespace redfish
{
+inline void
+ afterIfMatchRequest(crow::App& app,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ crow::Request& req, const std::string& ifMatchHeader,
+ const crow::Response& resIn)
+{
+ std::string computedEtag = resIn.computeEtag();
+ BMCWEB_LOG_DEBUG << "User provided if-match etag " << ifMatchHeader
+ << " computed etag " << computedEtag;
+ if (computedEtag != ifMatchHeader)
+ {
+ messages::preconditionFailed(asyncResp->res);
+ return;
+ }
+ // Restart the request without if-match
+ req.req.erase(boost::beast::http::field::if_match);
+ BMCWEB_LOG_DEBUG << "Restarting request";
+ app.handle(req, asyncResp);
+}
+
+inline bool handleIfMatch(crow::App& app, const crow::Request& req,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
+{
+ if (req.session == nullptr)
+ {
+ // If the user isn't authenticated, don't even attempt to parse match
+ // parameters
+ return true;
+ }
+
+ std::string ifMatch{
+ req.getHeaderValue(boost::beast::http::field::if_match)};
+ if (ifMatch.empty())
+ {
+ // No If-Match header. Nothing to do
+ return true;
+ }
+ if (req.req.method() != boost::beast::http::verb::patch &&
+ req.req.method() != boost::beast::http::verb::post &&
+ req.req.method() != boost::beast::http::verb::delete_)
+ {
+ messages::preconditionFailed(asyncResp->res);
+ return false;
+ }
+ boost::system::error_code ec;
+
+ // Try to GET the same resource
+ crow::Request newReq({boost::beast::http::verb::get, req.url, 11}, ec);
+
+ if (ec)
+ {
+ messages::internalError(asyncResp->res);
+ return false;
+ }
+
+ // New request has the same credentials as the old request
+ newReq.session = req.session;
+
+ // Construct a new response object to fill in, and check the hash of before
+ // we modify the Resource.
+ std::shared_ptr<bmcweb::AsyncResp> getReqAsyncResp =
+ std::make_shared<bmcweb::AsyncResp>();
+
+ getReqAsyncResp->res.setCompleteRequestHandler(std::bind_front(
+ afterIfMatchRequest, std::ref(app), asyncResp, req, ifMatch));
+
+ app.handle(newReq, getReqAsyncResp);
+ return false;
+}
// Sets up the Redfish Route and delegates some of the query parameter
// processing. |queryCapabilities| stores which query parameters will be
@@ -64,6 +133,11 @@ namespace redfish
return false;
}
+ if (!handleIfMatch(app, req, asyncResp))
+ {
+ return false;
+ }
+
bool needToCallHandlers = true;
#ifdef BMCWEB_ENABLE_REDFISH_AGGREGATION