Add 'reason' message to websocket close method
Now websocket client receives a proper reason from a server.
Removed filling asyncResp from onopen() method from nbdproxy.h
because it was redundant. Websocket does not response to client
using asyncResp.
Removed close from NbdProxyServer destructor because it is always
called in onclose() method.
Tested:
- Mounted and unmounted virtual media using proxy mode few times
as administrator with success.
- Mounted virtual media using proxy mode as operator and receives
proper reason on client side.
- Verify if errors are received properly on client side when
mounting operation fails.
Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>
Change-Id: If3b1cc9782de71a0975416872bc2fe8e3824148a
diff --git a/http/websocket.h b/http/websocket.h
index b89a74b..91b537b 100644
--- a/http/websocket.h
+++ b/http/websocket.h
@@ -166,7 +166,7 @@
void close(const std::string_view msg) override
{
ws.async_close(
- boost::beast::websocket::close_code::normal,
+ {boost::beast::websocket::close_code::normal, msg},
[self(shared_from_this())](boost::system::error_code ec) {
if (ec == boost::asio::error::operation_aborted)
{
diff --git a/include/nbd_proxy.hpp b/include/nbd_proxy.hpp
index 0356161..212c1db 100644
--- a/include/nbd_proxy.hpp
+++ b/include/nbd_proxy.hpp
@@ -52,7 +52,6 @@
~NbdProxyServer()
{
BMCWEB_LOG_DEBUG << "NbdProxyServer destructor";
- close();
}
std::string getEndpointId() const
@@ -89,12 +88,14 @@
doWrite();
});
- auto mountHandler = [](const boost::system::error_code ec,
- const bool status) {
+ auto mountHandler = [this, self(shared_from_this())](
+ const boost::system::error_code ec,
+ const bool status) {
if (ec)
{
BMCWEB_LOG_ERROR << "DBus error: cannot call mount method = "
<< ec.message();
+ connection.close("Failed to mount media");
return;
}
};
@@ -255,163 +256,162 @@
std::shared_ptr<bmcweb::AsyncResp> asyncResp) {
BMCWEB_LOG_DEBUG << "nbd-proxy.onopen(" << &conn << ")";
- auto getUserInfoHandler = [&conn, asyncResp{std::move(asyncResp)}](
- const boost::system::error_code ec,
- boost::container::flat_map<
- std::string,
- std::variant<
- bool, std::string,
+ auto getUserInfoHandler =
+ [&conn, asyncResp](
+ const boost::system::error_code ec,
+ boost::container::flat_map<
+ std::string, std::variant<bool, std::string,
std::vector<std::string>>>
- userInfo) {
- if (ec)
- {
- BMCWEB_LOG_ERROR << "GetUserInfo failed...";
- asyncResp->res.result(
- boost::beast::http::status::internal_server_error);
- return;
- }
-
- const std::string* userRolePtr = nullptr;
- auto userInfoIter = userInfo.find("UserPrivilege");
- if (userInfoIter != userInfo.end())
- {
- userRolePtr =
- std::get_if<std::string>(&userInfoIter->second);
- }
-
- std::string userRole{};
- if (userRolePtr != nullptr)
- {
- userRole = *userRolePtr;
- BMCWEB_LOG_DEBUG << "userName = " << conn.getUserName()
- << " userRole = " << *userRolePtr;
- }
-
- // Get the user privileges from the role
- ::redfish::Privileges userPrivileges =
- ::redfish::getUserPrivileges(userRole);
-
- const ::redfish::Privileges requiredPrivileges{
- requiredPrivilegeString};
-
- if (!userPrivileges.isSupersetOf(requiredPrivileges))
- {
- BMCWEB_LOG_DEBUG << "User " << conn.getUserName()
- << " not authorized for nbd connection";
- asyncResp->res.result(
- boost::beast::http::status::unauthorized);
- return;
- }
-
- for (const auto session : sessions)
- {
- if (session.second->getEndpointId() == conn.req.target())
- {
- BMCWEB_LOG_ERROR
- << "Cannot open new connection - socket is in use";
- asyncResp->res.result(
- boost::beast::http::status::bad_request);
- return;
- }
- }
-
- auto openHandler = [asyncResp,
- &conn](const boost::system::error_code ec,
- dbus::utility::ManagedObjectType&
- objects) {
- const std::string* socketValue = nullptr;
- const std::string* endpointValue = nullptr;
- const std::string* endpointObjectPath = nullptr;
-
+ userInfo) {
if (ec)
{
- BMCWEB_LOG_ERROR << "DBus error: " << ec.message();
- asyncResp->res.result(
- boost::beast::http::status::internal_server_error);
+ BMCWEB_LOG_ERROR << "GetUserInfo failed...";
+ conn.close("Failed to get user information");
return;
}
- for (const auto& objectPath : objects)
+ const std::string* userRolePtr = nullptr;
+ auto userInfoIter = userInfo.find("UserPrivilege");
+ if (userInfoIter != userInfo.end())
{
- const auto interfaceMap = objectPath.second.find(
- "xyz.openbmc_project.VirtualMedia.MountPoint");
+ userRolePtr =
+ std::get_if<std::string>(&userInfoIter->second);
+ }
- if (interfaceMap == objectPath.second.end())
+ std::string userRole{};
+ if (userRolePtr != nullptr)
+ {
+ userRole = *userRolePtr;
+ BMCWEB_LOG_DEBUG << "userName = " << conn.getUserName()
+ << " userRole = " << *userRolePtr;
+ }
+
+ // Get the user privileges from the role
+ ::redfish::Privileges userPrivileges =
+ ::redfish::getUserPrivileges(userRole);
+
+ const ::redfish::Privileges requiredPrivileges{
+ requiredPrivilegeString};
+
+ if (!userPrivileges.isSupersetOf(requiredPrivileges))
+ {
+ BMCWEB_LOG_DEBUG
+ << "User " << conn.getUserName()
+ << " not authorized for nbd connection";
+ conn.close("Unathourized access");
+ return;
+ }
+
+ auto openHandler = [&conn, asyncResp](
+ const boost::system::error_code ec,
+ const dbus::utility::
+ ManagedObjectType& objects) {
+ const std::string* socketValue = nullptr;
+ const std::string* endpointValue = nullptr;
+ const std::string* endpointObjectPath = nullptr;
+
+ if (ec)
{
- BMCWEB_LOG_DEBUG << "Cannot find MountPoint object";
- continue;
+ BMCWEB_LOG_ERROR << "DBus error: " << ec.message();
+ conn.close("Failed to create mount point");
+ return;
}
- const auto endpoint =
- interfaceMap->second.find("EndpointId");
- if (endpoint == interfaceMap->second.end())
+ for (const auto& objectPath : objects)
{
- BMCWEB_LOG_DEBUG
- << "Cannot find EndpointId property";
- continue;
- }
+ const auto interfaceMap = objectPath.second.find(
+ "xyz.openbmc_project.VirtualMedia.MountPoint");
- endpointValue =
- std::get_if<std::string>(&endpoint->second);
-
- if (endpointValue == nullptr)
- {
- BMCWEB_LOG_ERROR
- << "EndpointId property value is null";
- continue;
- }
-
- if (*endpointValue == conn.req.target())
- {
- const auto socket =
- interfaceMap->second.find("Socket");
- if (socket == interfaceMap->second.end())
+ if (interfaceMap == objectPath.second.end())
{
BMCWEB_LOG_DEBUG
- << "Cannot find Socket property";
+ << "Cannot find MountPoint object";
continue;
}
- socketValue =
- std::get_if<std::string>(&socket->second);
- if (socketValue == nullptr)
+ const auto endpoint =
+ interfaceMap->second.find("EndpointId");
+ if (endpoint == interfaceMap->second.end())
+ {
+ BMCWEB_LOG_DEBUG
+ << "Cannot find EndpointId property";
+ continue;
+ }
+
+ endpointValue =
+ std::get_if<std::string>(&endpoint->second);
+
+ if (endpointValue == nullptr)
{
BMCWEB_LOG_ERROR
- << "Socket property value is null";
+ << "EndpointId property value is null";
continue;
}
- endpointObjectPath = &objectPath.first.str;
- break;
+ if (*endpointValue == conn.req.target())
+ {
+ const auto socket =
+ interfaceMap->second.find("Socket");
+ if (socket == interfaceMap->second.end())
+ {
+ BMCWEB_LOG_DEBUG
+ << "Cannot find Socket property";
+ continue;
+ }
+
+ socketValue =
+ std::get_if<std::string>(&socket->second);
+ if (socketValue == nullptr)
+ {
+ BMCWEB_LOG_ERROR
+ << "Socket property value is null";
+ continue;
+ }
+
+ endpointObjectPath = &objectPath.first.str;
+ break;
+ }
}
- }
- if (endpointObjectPath == nullptr)
- {
- BMCWEB_LOG_ERROR << "Cannot find requested EndpointId";
- asyncResp->res.result(
- boost::beast::http::status::not_found);
- return;
- }
+ if (objects.empty() || endpointObjectPath == nullptr)
+ {
+ BMCWEB_LOG_ERROR
+ << "Cannot find requested EndpointId";
+ conn.close("Failed to match EndpointId");
+ return;
+ }
- // If the socket file exists (i.e. after bmcweb crash),
- // we cannot reuse it.
- std::remove((*socketValue).c_str());
+ for (const auto& session : sessions)
+ {
+ if (session.second->getEndpointId() ==
+ conn.req.target())
+ {
+ BMCWEB_LOG_ERROR
+ << "Cannot open new connection - socket is "
+ "in use";
+ conn.close("Slot is in use");
+ return;
+ }
+ }
- sessions[&conn] = std::make_shared<NbdProxyServer>(
- conn, std::move(*socketValue),
- std::move(*endpointValue),
- std::move(*endpointObjectPath));
+ // If the socket file exists (i.e. after bmcweb crash),
+ // we cannot reuse it.
+ std::remove((*socketValue).c_str());
- sessions[&conn]->run();
+ sessions[&conn] = std::make_shared<NbdProxyServer>(
+ conn, std::move(*socketValue),
+ std::move(*endpointValue),
+ std::move(*endpointObjectPath));
- asyncResp->res.result(boost::beast::http::status::ok);
+ sessions[&conn]->run();
+ };
+ crow::connections::systemBus->async_method_call(
+ std::move(openHandler),
+ "xyz.openbmc_project.VirtualMedia",
+ "/xyz/openbmc_project/VirtualMedia",
+ "org.freedesktop.DBus.ObjectManager",
+ "GetManagedObjects");
};
- crow::connections::systemBus->async_method_call(
- std::move(openHandler), "xyz.openbmc_project.VirtualMedia",
- "/xyz/openbmc_project/VirtualMedia",
- "org.freedesktop.DBus.ObjectManager", "GetManagedObjects");
- };
crow::connections::systemBus->async_method_call(
std::move(getUserInfoHandler),
@@ -429,9 +429,9 @@
BMCWEB_LOG_DEBUG << "No session to close";
return;
}
- session->second->close();
// Remove reference to session in global map
sessions.erase(session);
+ session->second->close();
})
.onmessage([](crow::websocket::Connection& conn,
const std::string& data, bool isBinary) {
@@ -447,7 +447,6 @@
return;
}
}
- conn.close();
});
}
} // namespace nbd_proxy