Limit SPeL getValue to SimpleEvaluationContext (#15521)

* Limit SPeL getValue to SimpleEvaluationContext

* Fix pytests
This commit is contained in:
Sriharsha Chintalapani 2024-03-11 18:44:11 -07:00 committed by GitHub
parent a36cd24472
commit 9a69daf35e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 33 additions and 12 deletions

View File

@ -45,6 +45,7 @@ import org.openmetadata.service.Entity;
import org.openmetadata.service.exception.CatalogExceptionMessage; import org.openmetadata.service.exception.CatalogExceptionMessage;
import org.openmetadata.service.util.JsonUtils; import org.openmetadata.service.util.JsonUtils;
import org.springframework.expression.Expression; import org.springframework.expression.Expression;
import org.springframework.expression.spel.support.SimpleEvaluationContext;
@Slf4j @Slf4j
public final class AlertUtil { public final class AlertUtil {
@ -56,8 +57,13 @@ public final class AlertUtil {
} }
Expression expression = parseExpression(condition); Expression expression = parseExpression(condition);
AlertsRuleEvaluator ruleEvaluator = new AlertsRuleEvaluator(null); AlertsRuleEvaluator ruleEvaluator = new AlertsRuleEvaluator(null);
SimpleEvaluationContext context =
SimpleEvaluationContext.forReadOnlyDataBinding()
.withInstanceMethods()
.withRootObject(ruleEvaluator)
.build();
try { try {
expression.getValue(ruleEvaluator, clz); expression.getValue(context, clz);
} catch (Exception exception) { } catch (Exception exception) {
// Remove unnecessary class details in the exception message // Remove unnecessary class details in the exception message
String message = String message =
@ -73,7 +79,12 @@ public final class AlertUtil {
String completeCondition = buildCompleteCondition(alertFilterRules); String completeCondition = buildCompleteCondition(alertFilterRules);
AlertsRuleEvaluator ruleEvaluator = new AlertsRuleEvaluator(changeEvent); AlertsRuleEvaluator ruleEvaluator = new AlertsRuleEvaluator(changeEvent);
Expression expression = parseExpression(completeCondition); Expression expression = parseExpression(completeCondition);
result = Boolean.TRUE.equals(expression.getValue(ruleEvaluator, Boolean.class)); SimpleEvaluationContext context =
SimpleEvaluationContext.forReadOnlyDataBinding()
.withInstanceMethods()
.withRootObject(ruleEvaluator)
.build();
result = Boolean.TRUE.equals(expression.getValue(context, Boolean.class));
LOG.debug("Alert evaluated as Result : {}", result); LOG.debug("Alert evaluated as Result : {}", result);
return result; return result;
} else { } else {

View File

@ -132,7 +132,9 @@ public class PolicyRepository extends EntityRepository<Policy> {
public static List<String> filterRedundantResources(List<String> resources) { public static List<String> filterRedundantResources(List<String> resources) {
// If ALL_RESOURCES are in the resource list, remove redundant resources specifically mentioned // If ALL_RESOURCES are in the resource list, remove redundant resources specifically mentioned
boolean containsAllResources = resources.stream().anyMatch(ALL_RESOURCES::equalsIgnoreCase); boolean containsAllResources = resources.stream().anyMatch(ALL_RESOURCES::equalsIgnoreCase);
return containsAllResources ? new ArrayList<>(List.of(ALL_RESOURCES)) : resources; return containsAllResources
? new ArrayList<>(List.of(ALL_RESOURCES))
: new ArrayList<>(resources);
} }
public static List<MetadataOperation> filterRedundantOperations( public static List<MetadataOperation> filterRedundantOperations(
@ -142,9 +144,7 @@ public class PolicyRepository extends EntityRepository<Policy> {
boolean containsViewAll = operations.stream().anyMatch(o -> o.equals(VIEW_ALL)); boolean containsViewAll = operations.stream().anyMatch(o -> o.equals(VIEW_ALL));
if (containsViewAll) { if (containsViewAll) {
operations = operations =
operations.stream() operations.stream().filter(o -> o.equals(VIEW_ALL) || !isViewOperation(o)).toList();
.filter(o -> o.equals(VIEW_ALL) || !isViewOperation(o))
.collect(Collectors.toList());
} }
// If EDIT_ALL is in the operation list, remove all the other specific edit operations that are // If EDIT_ALL is in the operation list, remove all the other specific edit operations that are
@ -152,11 +152,9 @@ public class PolicyRepository extends EntityRepository<Policy> {
boolean containsEditAll = operations.stream().anyMatch(o -> o.equals(EDIT_ALL)); boolean containsEditAll = operations.stream().anyMatch(o -> o.equals(EDIT_ALL));
if (containsEditAll) { if (containsEditAll) {
operations = operations =
operations.stream() operations.stream().filter(o -> o.equals(EDIT_ALL) || !isEditOperation(o)).toList();
.filter(o -> o.equals(EDIT_ALL) || !isEditOperation(o))
.collect(Collectors.toList());
} }
return operations; return new ArrayList<>(operations);
} }
/** Handles entity updated from PUT and POST operation. */ /** Handles entity updated from PUT and POST operation. */

View File

@ -498,6 +498,7 @@ public class PolicyResource extends EntityResource<Policy, PolicyRepository> {
@Parameter(description = "Expression of validating rule", schema = @Schema(type = "string")) @Parameter(description = "Expression of validating rule", schema = @Schema(type = "string"))
@PathParam("expression") @PathParam("expression")
String expression) { String expression) {
authorizer.authorizeAdmin(securityContext);
CompiledRule.validateExpression(expression, Boolean.class); CompiledRule.validateExpression(expression, Boolean.class);
} }

View File

@ -18,6 +18,7 @@ import org.openmetadata.service.security.AuthorizationException;
import org.openmetadata.service.security.policyevaluator.SubjectContext.PolicyContext; import org.openmetadata.service.security.policyevaluator.SubjectContext.PolicyContext;
import org.springframework.expression.Expression; import org.springframework.expression.Expression;
import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.SimpleEvaluationContext;
/** This class is used in a single threaded model and hence does not have concurrency support */ /** This class is used in a single threaded model and hence does not have concurrency support */
@Slf4j @Slf4j
@ -54,8 +55,13 @@ public class CompiledRule extends Rule {
} }
Expression expression = parseExpression(condition); Expression expression = parseExpression(condition);
RuleEvaluator ruleEvaluator = new RuleEvaluator(); RuleEvaluator ruleEvaluator = new RuleEvaluator();
SimpleEvaluationContext context =
SimpleEvaluationContext.forReadOnlyDataBinding()
.withInstanceMethods()
.withRootObject(ruleEvaluator)
.build();
try { try {
expression.getValue(ruleEvaluator, clz); expression.getValue(context, clz);
} catch (Exception exception) { } catch (Exception exception) {
// Remove unnecessary class details in the exception message // Remove unnecessary class details in the exception message
String message = String message =
@ -216,7 +222,12 @@ public class CompiledRule extends Rule {
return true; return true;
} }
RuleEvaluator ruleEvaluator = new RuleEvaluator(policyContext, subjectContext, resourceContext); RuleEvaluator ruleEvaluator = new RuleEvaluator(policyContext, subjectContext, resourceContext);
return Boolean.TRUE.equals(expr.getValue(ruleEvaluator, Boolean.class)); SimpleEvaluationContext context =
SimpleEvaluationContext.forReadOnlyDataBinding()
.withInstanceMethods()
.withRootObject(ruleEvaluator)
.build();
return Boolean.TRUE.equals(expr.getValue(context, Boolean.class));
} }
public static boolean overrideAccess(Access newAccess, Access currentAccess) { public static boolean overrideAccess(Access newAccess, Access currentAccess) {