Skip to content

Commit

Permalink
FilterUtility: Replace some nested raw pointers by our unique_ptr<X>*
Browse files Browse the repository at this point in the history
  • Loading branch information
@yhabteab
yhabteab committed Oct 17, 2022
1 parent dd7009d commit e0155c7
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 24 deletions.
29 changes: 17 additions & 12 deletions lib/remote/filterutility.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ static void FilteredAddTarget(ScriptFrame& permissionFrame, Expression *permissi
*
* @return bool
*/
bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permission, Expression **permissionFilter)
bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permission, std::unique_ptr<Expression>* permissionFilter)
{
if (permissionFilter)
*permissionFilter = nullptr;
Expand All @@ -149,6 +149,7 @@ bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permis
bool foundPermission = false;
String requiredPermission = permission.ToLower();

std::unique_ptr<Expression> filtersBuffer;
Array::Ptr permissions = user->GetPermissions();
if (permissions) {
ObjectLock olock(permissions);
Expand All @@ -175,14 +176,18 @@ bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permis
std::unique_ptr<Expression> indexer{new IndexerExpression(std::unique_ptr<Expression>(MakeLiteral(filter)), std::unique_ptr<Expression>(MakeLiteral("call")))};
FunctionCallExpression *fexpr = new FunctionCallExpression(std::move(indexer), std::move(args));

if (!*permissionFilter)
*permissionFilter = fexpr;
if (!filtersBuffer)
filtersBuffer.reset(fexpr);
else
*permissionFilter = new LogicalOrExpression(std::unique_ptr<Expression>(*permissionFilter), std::unique_ptr<Expression>(fexpr));
filtersBuffer = std::make_unique<LogicalOrExpression>(std::move(filtersBuffer), std::unique_ptr<Expression>(fexpr));
}
}
}

if (permissionFilter) {
*permissionFilter = std::move(filtersBuffer);
}

if (!foundPermission) {
Log(LogWarning, "FilterUtility")
<< "Missing permission: " << requiredPermission;
Expand All @@ -191,7 +196,7 @@ bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permis
return foundPermission;
}

void FilterUtility::CheckPermission(const ApiUser::Ptr& user, const String& permission, Expression **permissionFilter)
void FilterUtility::CheckPermission(const ApiUser::Ptr& user, const String& permission, std::unique_ptr<Expression>* permissionFilter)
{
if (!HasPermission(user, permission, permissionFilter)) {
BOOST_THROW_EXCEPTION(ScriptError("Missing permission: " + permission.ToLower()));
Expand All @@ -209,7 +214,7 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c
else
provider = new ConfigObjectTargetProvider();

Expression *permissionFilter;
std::unique_ptr<Expression> permissionFilter;
CheckPermission(user, qd.Permission, &permissionFilter);

Namespace::Ptr permissionFrameNS = new Namespace();
Expand All @@ -226,7 +231,7 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c
String name = HttpUtility::GetLastParameter(query, attr);
Object::Ptr target = provider->GetTargetByName(type, name);

if (!FilterUtility::EvaluateFilter(permissionFrame, permissionFilter, target, variableName))
if (!FilterUtility::EvaluateFilter(permissionFrame, permissionFilter.get(), target, variableName))
BOOST_THROW_EXCEPTION(ScriptError("Access denied to object '" + name + "' of type '" + type + "'"));

result.emplace_back(std::move(target));
Expand All @@ -242,7 +247,7 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c
for (const String& name : names) {
Object::Ptr target = provider->GetTargetByName(type, name);

if (!FilterUtility::EvaluateFilter(permissionFrame, permissionFilter, target, variableName))
if (!FilterUtility::EvaluateFilter(permissionFrame, permissionFilter.get(), target, variableName))
BOOST_THROW_EXCEPTION(ScriptError("Access denied to object '" + name + "' of type '" + type + "'"));

result.emplace_back(std::move(target));
Expand Down Expand Up @@ -279,15 +284,15 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c
}
}

provider->FindTargets(type, [&permissionFrame, permissionFilter, &frame, &ufilter, &result, variableName](const Object::Ptr& target) {
FilteredAddTarget(permissionFrame, permissionFilter, frame, &*ufilter, result, variableName, target);
provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &ufilter, &result, variableName](const Object::Ptr& target) {
FilteredAddTarget(permissionFrame, permissionFilter.get(), frame, &*ufilter, result, variableName, target);
});
} else {
/* Ensure to pass a nullptr as filter expression.
* GCC 8.1.1 on F28 causes problems, see GH #6533.
*/
provider->FindTargets(type, [&permissionFrame, permissionFilter, &frame, &result, variableName](const Object::Ptr& target) {
FilteredAddTarget(permissionFrame, permissionFilter, frame, nullptr, result, variableName, target);
provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &result, variableName](const Object::Ptr& target) {
FilteredAddTarget(permissionFrame, permissionFilter.get(), frame, nullptr, result, variableName, target);
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/remote/filterutility.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ 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 void CheckPermission(const ApiUser::Ptr& user, const String& permission, std::unique_ptr<Expression>* filter = nullptr);
static bool HasPermission(const ApiUser::Ptr& user, const String& permission, std::unique_ptr<Expression>* permissionFilter = nullptr);
static std::vector<Value> GetFilterTargets(const QueryDescription& qd, const Dictionary::Ptr& query,
const ApiUser::Ptr& user, const String& variableName = String());
static bool EvaluateFilter(ScriptFrame& frame, Expression *filter,
Expand Down
20 changes: 10 additions & 10 deletions lib/remote/objectqueryhandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,10 @@ bool ObjectQueryHandler::HandleRequest(
joinAttrs.insert(field.Name);
}

std::unordered_map<Type*, std::pair<bool, Expression::Ptr>> typePermissions;
typedef std::unordered_map<Type*, std::pair<bool, std::unique_ptr<Expression>>> TypePermissionPairs;

TypePermissionPairs typePermissions;
TypePermissionPairs::iterator permissionFilters;
std::unordered_map<Object*, bool> objectAccessAllowed;

for (const ConfigObject::Ptr& obj : objs) {
Expand Down Expand Up @@ -262,24 +265,21 @@ bool ObjectQueryHandler::HandleRequest(
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;
std::unique_ptr<Expression> permissionFilter;
granted = FilterUtility::HasPermission(user, permission, &permissionFilter);

typePermissions.insert({reflectionType.get(), std::make_pair(granted, permissionFilter)});
permissionFilters = typePermissions.insert({reflectionType.get(), std::make_pair(granted, std::move(permissionFilter))}).first;
} else {
std::tie(granted, permissionFilter) = it->second;
permissionFilters = it;
}

if (!granted) {
if (!permissionFilters->second.first) {
// Not authorized
continue;
}
Expand All @@ -291,7 +291,7 @@ bool ObjectQueryHandler::HandleRequest(
ScriptFrame permissionFrame(false, new Namespace());

try {
accessAllowed = FilterUtility::EvaluateFilter(permissionFrame, permissionFilter.get(), joinedObj);
accessAllowed = FilterUtility::EvaluateFilter(permissionFrame, permissionFilters->second.second.get(), joinedObj);
} catch (const ScriptError& err) {
accessAllowed = false;
}
Expand Down

0 comments on commit e0155c7

Please sign in to comment.