From 3b80153848c113c1bbfad42c691ef73962698dcd Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 15 Sep 2025 11:17:45 +0200 Subject: [PATCH] Remove `AuthenticatedApiUser` thread-local variable & pass it as param instead --- lib/icinga/apiactions.cpp | 109 +++++++++++++++++++++++----------- lib/icinga/apiactions.hpp | 28 ++++----- lib/remote/actionshandler.cpp | 9 +-- lib/remote/actionshandler.hpp | 2 - lib/remote/apiaction.cpp | 4 +- lib/remote/apiaction.hpp | 5 +- 6 files changed, 94 insertions(+), 63 deletions(-) diff --git a/lib/icinga/apiactions.cpp b/lib/icinga/apiactions.cpp index a1f89e338..a5af819b7 100644 --- a/lib/icinga/apiactions.cpp +++ b/lib/icinga/apiactions.cpp @@ -54,8 +54,11 @@ Dictionary::Ptr ApiActions::CreateResult(int code, const String& status, return result; } -Dictionary::Ptr ApiActions::ProcessCheckResult(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::ProcessCheckResult( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { using Result = Checkable::ProcessingResult; @@ -141,8 +144,11 @@ Dictionary::Ptr ApiActions::ProcessCheckResult(const ConfigObject::Ptr& object, return ApiActions::CreateResult(500, "Unexpected result (" + std::to_string(static_cast(result)) + ") for object '" + checkable->GetName() + "'. Please submit a bug report at https://github.com/Icinga/icinga2"); } -Dictionary::Ptr ApiActions::RescheduleCheck(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::RescheduleCheck( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -166,8 +172,11 @@ Dictionary::Ptr ApiActions::RescheduleCheck(const ConfigObject::Ptr& object, return ApiActions::CreateResult(200, "Successfully rescheduled check for object '" + checkable->GetName() + "'."); } -Dictionary::Ptr ApiActions::SendCustomNotification(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::SendCustomNotification( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -189,8 +198,11 @@ Dictionary::Ptr ApiActions::SendCustomNotification(const ConfigObject::Ptr& obje return ApiActions::CreateResult(200, "Successfully sent custom notification for object '" + checkable->GetName() + "'."); } -Dictionary::Ptr ApiActions::DelayNotification(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::DelayNotification( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -207,8 +219,11 @@ Dictionary::Ptr ApiActions::DelayNotification(const ConfigObject::Ptr& object, return ApiActions::CreateResult(200, "Successfully delayed notifications for object '" + checkable->GetName() + "'."); } -Dictionary::Ptr ApiActions::AcknowledgeProblem(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::AcknowledgeProblem( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -269,8 +284,11 @@ Dictionary::Ptr ApiActions::AcknowledgeProblem(const ConfigObject::Ptr& object, return ApiActions::CreateResult(200, "Successfully acknowledged problem for object '" + checkable->GetName() + "'."); } -Dictionary::Ptr ApiActions::RemoveAcknowledgement(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::RemoveAcknowledgement( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -293,8 +311,11 @@ Dictionary::Ptr ApiActions::RemoveAcknowledgement(const ConfigObject::Ptr& objec return ApiActions::CreateResult(200, "Successfully removed acknowledgement for object '" + checkable->GetName() + "'."); } -Dictionary::Ptr ApiActions::AddComment(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::AddComment( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -332,8 +353,11 @@ Dictionary::Ptr ApiActions::AddComment(const ConfigObject::Ptr& object, + "'.", additional); } -Dictionary::Ptr ApiActions::RemoveComment(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::RemoveComment( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { ConfigObjectsSharedLock lock (std::try_to_lock); @@ -366,8 +390,11 @@ Dictionary::Ptr ApiActions::RemoveComment(const ConfigObject::Ptr& object, return ApiActions::CreateResult(200, "Successfully removed comment '" + commentName + "'."); } -Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::ScheduleDowntime( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -536,8 +563,11 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object, downtimeName + "' for object '" + checkable->GetName() + "'.", additional); } -Dictionary::Ptr ApiActions::RemoveDowntime(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::RemoveDowntime( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { ConfigObjectsSharedLock lock (std::try_to_lock); @@ -589,24 +619,29 @@ Dictionary::Ptr ApiActions::RemoveDowntime(const ConfigObject::Ptr& object, } } -Dictionary::Ptr ApiActions::ShutdownProcess(const ConfigObject::Ptr&, - [[maybe_unused]] const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::ShutdownProcess( + const ConfigObject::Ptr&, + const ApiUser::Ptr&, + [[maybe_unused]] const Dictionary::Ptr& params +) { Application::RequestShutdown(); return ApiActions::CreateResult(200, "Shutting down Icinga 2."); } -Dictionary::Ptr ApiActions::RestartProcess(const ConfigObject::Ptr&, - [[maybe_unused]] const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::RestartProcess( + const ConfigObject::Ptr&, + const ApiUser::Ptr&, + [[maybe_unused]] const Dictionary::Ptr& params +) { Application::RequestRestart(); return ApiActions::CreateResult(200, "Restarting Icinga 2."); } -Dictionary::Ptr ApiActions::GenerateTicket(const ConfigObject::Ptr&, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::GenerateTicket(const ConfigObject::Ptr&, const ApiUser::Ptr&, const Dictionary::Ptr& params) { if (!params->Contains("cn")) return ApiActions::CreateResult(400, "Option 'cn' is required"); @@ -654,7 +689,11 @@ Value ApiActions::GetSingleObjectByNameUsingPermissions(const String& type, cons return objs.at(0); }; -Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::ExecuteCommand( + const ConfigObject::Ptr& object, + const ApiUser::Ptr& apiUser, + const Dictionary::Ptr& params +) { ApiListener::Ptr listener = ApiListener::GetInstance(); @@ -718,11 +757,11 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons nullptr, MacroProcessor::EscapeCallback(), nullptr, false ); - if (!ActionsHandler::AuthenticatedApiUser) + if (!apiUser) BOOST_THROW_EXCEPTION(std::invalid_argument("Can't find API user.")); /* Get endpoint */ - Endpoint::Ptr endpointPtr = GetSingleObjectByNameUsingPermissions(Endpoint::GetTypeName(), resolved_endpoint, ActionsHandler::AuthenticatedApiUser); + Endpoint::Ptr endpointPtr = GetSingleObjectByNameUsingPermissions(Endpoint::GetTypeName(), resolved_endpoint, apiUser); if (!endpointPtr) return ApiActions::CreateResult(404, "Can't find a valid endpoint for '" + resolved_endpoint + "'."); @@ -780,7 +819,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons Dictionary::Ptr execParams = new Dictionary(); if (command_type == "CheckCommand") { - CheckCommand::Ptr cmd = GetSingleObjectByNameUsingPermissions(CheckCommand::GetTypeName(), resolved_command, ActionsHandler::AuthenticatedApiUser); + CheckCommand::Ptr cmd = GetSingleObjectByNameUsingPermissions(CheckCommand::GetTypeName(), resolved_command, apiUser); if (!cmd) return ApiActions::CreateResult(404, "Can't find a valid " + command_type + " for '" + resolved_command + "'."); @@ -792,7 +831,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons cmd->Execute(checkable, cr, listener->GetWaitGroup(), execMacros, false); } } else if (command_type == "EventCommand") { - EventCommand::Ptr cmd = GetSingleObjectByNameUsingPermissions(EventCommand::GetTypeName(), resolved_command, ActionsHandler::AuthenticatedApiUser); + EventCommand::Ptr cmd = GetSingleObjectByNameUsingPermissions(EventCommand::GetTypeName(), resolved_command, apiUser); if (!cmd) return ApiActions::CreateResult(404, "Can't find a valid " + command_type + " for '" + resolved_command + "'."); @@ -804,7 +843,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons cmd->Execute(checkable, execMacros, false); } } else if (command_type == "NotificationCommand") { - NotificationCommand::Ptr cmd = GetSingleObjectByNameUsingPermissions(NotificationCommand::GetTypeName(), resolved_command, ActionsHandler::AuthenticatedApiUser); + NotificationCommand::Ptr cmd = GetSingleObjectByNameUsingPermissions(NotificationCommand::GetTypeName(), resolved_command, apiUser); if (!cmd) return ApiActions::CreateResult(404, "Can't find a valid " + command_type + " for '" + resolved_command + "'."); @@ -821,7 +860,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons MacroProcessor::EscapeCallback(), nullptr, false ); - User::Ptr user = GetSingleObjectByNameUsingPermissions(User::GetTypeName(), resolved_user, ActionsHandler::AuthenticatedApiUser); + User::Ptr user = GetSingleObjectByNameUsingPermissions(User::GetTypeName(), resolved_user, apiUser); if (!user) return ApiActions::CreateResult(404, "Can't find a valid user for '" + resolved_user + "'."); @@ -840,7 +879,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons MacroProcessor::EscapeCallback(), nullptr, false ); - Notification::Ptr notification = GetSingleObjectByNameUsingPermissions(Notification::GetTypeName(), resolved_notification, ActionsHandler::AuthenticatedApiUser); + Notification::Ptr notification = GetSingleObjectByNameUsingPermissions(Notification::GetTypeName(), resolved_notification, apiUser); if (!notification) return ApiActions::CreateResult(404, "Can't find a valid notification for '" + resolved_notification + "'."); @@ -853,7 +892,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons }); cmd->Execute(notification, user, cr, NotificationType::NotificationCustom, - ActionsHandler::AuthenticatedApiUser->GetName(), "", execMacros, false); + apiUser->GetName(), "", execMacros, false); } } diff --git a/lib/icinga/apiactions.hpp b/lib/icinga/apiactions.hpp index db9028ac1..9cf34028c 100644 --- a/lib/icinga/apiactions.hpp +++ b/lib/icinga/apiactions.hpp @@ -18,20 +18,20 @@ namespace icinga class ApiActions { public: - static Dictionary::Ptr ProcessCheckResult(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr RescheduleCheck(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr SendCustomNotification(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr DelayNotification(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr AcknowledgeProblem(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr RemoveAcknowledgement(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr AddComment(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr RemoveComment(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr ScheduleDowntime(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr RemoveDowntime(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr ShutdownProcess(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr RestartProcess(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr GenerateTicket(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr ExecuteCommand(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); + static Dictionary::Ptr ProcessCheckResult(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr RescheduleCheck(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr SendCustomNotification(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr DelayNotification(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr AcknowledgeProblem(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr RemoveAcknowledgement(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr AddComment(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr RemoveComment(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr ScheduleDowntime(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr RemoveDowntime(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr ShutdownProcess(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr RestartProcess(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr GenerateTicket(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); + static Dictionary::Ptr ExecuteCommand(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); private: static Dictionary::Ptr CreateResult(int code, const String& status, const Dictionary::Ptr& additional = nullptr); diff --git a/lib/remote/actionshandler.cpp b/lib/remote/actionshandler.cpp index 8dde35987..e2251d342 100644 --- a/lib/remote/actionshandler.cpp +++ b/lib/remote/actionshandler.cpp @@ -12,8 +12,6 @@ using namespace icinga; -thread_local ApiUser::Ptr ActionsHandler::AuthenticatedApiUser; - REGISTER_URLHANDLER("/v1/actions", ActionsHandler); bool ActionsHandler::HandleRequest( @@ -80,11 +78,6 @@ bool ActionsHandler::HandleRequest( bool verbose = false; - ActionsHandler::AuthenticatedApiUser = user; - Defer a ([]() { - ActionsHandler::AuthenticatedApiUser = nullptr; - }); - if (params) verbose = HttpUtility::GetLastParameter(params, "verbose"); @@ -111,7 +104,7 @@ bool ActionsHandler::HandleRequest( } try { - results.emplace_back(action->Invoke(obj, params)); + results.emplace_back(action->Invoke(obj, user, params)); } catch (const std::exception& ex) { Dictionary::Ptr fail = new Dictionary({ { "code", 500 }, diff --git a/lib/remote/actionshandler.hpp b/lib/remote/actionshandler.hpp index c23d25324..c3369eaf2 100644 --- a/lib/remote/actionshandler.hpp +++ b/lib/remote/actionshandler.hpp @@ -14,8 +14,6 @@ class ActionsHandler final : public HttpHandler public: DECLARE_PTR_TYPEDEFS(ActionsHandler); - static thread_local ApiUser::Ptr AuthenticatedApiUser; - bool HandleRequest( const WaitGroup::Ptr& waitGroup, const HttpApiRequest& request, diff --git a/lib/remote/apiaction.cpp b/lib/remote/apiaction.cpp index f50d265bd..d3d2cfac4 100644 --- a/lib/remote/apiaction.cpp +++ b/lib/remote/apiaction.cpp @@ -13,9 +13,9 @@ ApiAction::ApiAction(std::vector types, Callback action) : m_Types(std::move(types)), m_Callback(std::move(action)) { } -Value ApiAction::Invoke(const ConfigObject::Ptr& target, const Dictionary::Ptr& params) +Value ApiAction::Invoke(const ConfigObject::Ptr& target, const ApiUser::Ptr& user, const Dictionary::Ptr& params) { - return m_Callback(target, params); + return m_Callback(target, user, params); } const std::vector& ApiAction::GetTypes() const diff --git a/lib/remote/apiaction.hpp b/lib/remote/apiaction.hpp index 8b822070f..94435e9f8 100644 --- a/lib/remote/apiaction.hpp +++ b/lib/remote/apiaction.hpp @@ -9,6 +9,7 @@ #include "base/value.hpp" #include "base/dictionary.hpp" #include "base/configobject.hpp" +#include "remote/apiuser.hpp" #include #include @@ -25,11 +26,11 @@ class ApiAction final : public Object public: DECLARE_PTR_TYPEDEFS(ApiAction); - typedef std::function Callback; + typedef std::function Callback; ApiAction(std::vector registerTypes, Callback function); - Value Invoke(const ConfigObject::Ptr& target, const Dictionary::Ptr& params); + Value Invoke(const ConfigObject::Ptr& target, const ApiUser::Ptr& user, const Dictionary::Ptr& params); const std::vector& GetTypes() const;