From 21b55cb1ac923da6779135116b67fec14d6d0aac Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 5 Oct 2022 17:49:02 +0200 Subject: [PATCH 1/3] FilterUtility: Outsource permission matching from CheckPermission() to a separate method --- lib/remote/filterutility.cpp | 27 ++++++++++++++++++++++++--- lib/remote/filterutility.hpp | 1 + 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/remote/filterutility.cpp b/lib/remote/filterutility.cpp index 3bc1578e7..9bcc60f71 100644 --- a/lib/remote/filterutility.cpp +++ b/lib/remote/filterutility.cpp @@ -124,13 +124,27 @@ static void FilteredAddTarget(ScriptFrame& permissionFrame, Expression *permissi } } -void FilterUtility::CheckPermission(const ApiUser::Ptr& user, const String& permission, Expression **permissionFilter) +/** + * Checks whether the given API user is granted the given permission + * + * When you desire an exception to be raised when the given user doesn't have the given permission, + * you need to use FilterUtility::CheckPermission(). + * + * @param user ApiUser pointer to the user object you want to check the permission of + * @param permission The actual permission you want to check the user permission against + * @param permissionFilter Expression pointer that is used as an output buffer for all the filter expressions of the + * individual permissions of the given user to be evaluated. It's up to the caller to delete + * this pointer when it's not needed any more. + * + * @return bool + */ +bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permission, Expression **permissionFilter) { if (permissionFilter) *permissionFilter = nullptr; if (permission.IsEmpty()) - return; + return true; bool foundPermission = false; String requiredPermission = permission.ToLower(); @@ -172,8 +186,15 @@ void FilterUtility::CheckPermission(const ApiUser::Ptr& user, const String& perm if (!foundPermission) { Log(LogWarning, "FilterUtility") << "Missing permission: " << requiredPermission; + } - BOOST_THROW_EXCEPTION(ScriptError("Missing permission: " + requiredPermission)); + return foundPermission; +} + +void FilterUtility::CheckPermission(const ApiUser::Ptr& user, const String& permission, Expression **permissionFilter) +{ + if (!HasPermission(user, permission, permissionFilter)) { + BOOST_THROW_EXCEPTION(ScriptError("Missing permission: " + permission.ToLower())); } } diff --git a/lib/remote/filterutility.hpp b/lib/remote/filterutility.hpp index 70e5b7e6f..1cebffc0b 100644 --- a/lib/remote/filterutility.hpp +++ b/lib/remote/filterutility.hpp @@ -52,6 +52,7 @@ class FilterUtility public: static Type::Ptr TypeFromPluralName(const String& pluralName); static void CheckPermission(const ApiUser::Ptr& user, const String& permission, Expression **filter = nullptr); + static bool HasPermission(const ApiUser::Ptr& user, const String& permission, Expression **permissionFilter = nullptr); static std::vector GetFilterTargets(const QueryDescription& qd, const Dictionary::Ptr& query, const ApiUser::Ptr& user, const String& variableName = String()); static bool EvaluateFilter(ScriptFrame& frame, Expression *filter, From 02524f59937e419dfa7af9b7cd2ccbcaa13ee4bf Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 5 Oct 2022 17:51:13 +0200 Subject: [PATCH 2/3] ObjectQueryHandler: Check user permissions on joined relations --- lib/remote/objectqueryhandler.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp index 3f827037c..c29b61ba4 100644 --- a/lib/remote/objectqueryhandler.cpp +++ b/lib/remote/objectqueryhandler.cpp @@ -8,6 +8,7 @@ #include "base/configtype.hpp" #include #include +#include using namespace icinga; @@ -189,6 +190,8 @@ bool ObjectQueryHandler::HandleRequest( joinAttrs.insert(field.Name); } + std::unordered_map> typePermissions; + for (const ConfigObject::Ptr& obj : objs) { DictionaryData result1{ { "name", obj->GetName() }, @@ -257,6 +260,29 @@ bool ObjectQueryHandler::HandleRequest( if (!joinedObj) continue; + Type::Ptr reflectionType = joinedObj->GetReflectionType(); + Expression::Ptr permissionFilter; + + auto it = typePermissions.find(reflectionType.get()); + bool granted; + + if (it == typePermissions.end()) { + String permission = "objects/query/" + reflectionType->GetName(); + + Expression *filter = nullptr; + granted = FilterUtility::HasPermission(user, permission, &filter); + permissionFilter = filter; + + typePermissions.insert({reflectionType.get(), std::make_pair(granted, permissionFilter)}); + } else { + std::tie(granted, permissionFilter) = it->second; + } + + if (!granted) { + // Not authorized + continue; + } + String prefix = field.NavigationName; try { From adc42e101d5ab2bbd3428e8676838dab11a53d90 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 5 Oct 2022 17:52:29 +0200 Subject: [PATCH 3/3] Evaluate permission filters also on all joined relations --- lib/remote/objectqueryhandler.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp index c29b61ba4..f059c0328 100644 --- a/lib/remote/objectqueryhandler.cpp +++ b/lib/remote/objectqueryhandler.cpp @@ -191,6 +191,7 @@ bool ObjectQueryHandler::HandleRequest( } std::unordered_map> typePermissions; + std::unordered_map objectAccessAllowed; for (const ConfigObject::Ptr& obj : objs) { DictionaryData result1{ @@ -283,6 +284,28 @@ bool ObjectQueryHandler::HandleRequest( continue; } + auto relation = objectAccessAllowed.find(joinedObj.get()); + bool accessAllowed; + + if (relation == objectAccessAllowed.end()) { + ScriptFrame permissionFrame(false, new Namespace()); + + try { + accessAllowed = FilterUtility::EvaluateFilter(permissionFrame, permissionFilter.get(), joinedObj); + } catch (const ScriptError& err) { + accessAllowed = false; + } + + objectAccessAllowed.insert({joinedObj.get(), accessAllowed}); + } else { + accessAllowed = relation->second; + } + + if (!accessAllowed) { + // Access denied + continue; + } + String prefix = field.NavigationName; try {