Permission visitor normalization (#2035)

* add normalization vistior for permission expression * Change PermissionToFilterPredicate to work on normalized expression * add copyright * fix checkstyle * add comments Co-authored-by: Chandrasekar Rajasekar <[email protected]> Co-authored-by: Aaron Klish <[email protected]>
yahoo · Apr 28, 2021 · e64bd8e · e64bd8e
1 parent 53b2333
commit e64bd8e
Show file tree

Hide file tree

Showing 14 changed files with 372 additions and 38 deletions.
diff --git a/.../src/main/java/com/yahoo/elide/core/security/permissions/PermissionExpressionBuilder.java b/.../src/main/java/com/yahoo/elide/core/security/permissions/PermissionExpressionBuilder.java
@@ -15,7 +15,6 @@
 import com.yahoo.elide.core.dictionary.EntityDictionary;
 import com.yahoo.elide.core.filter.expression.FilterExpression;
 import com.yahoo.elide.core.filter.expression.OrFilterExpression;
-import com.yahoo.elide.core.filter.visitors.FilterExpressionNormalizationVisitor;
 import com.yahoo.elide.core.security.ChangeSpec;
 import com.yahoo.elide.core.security.CheckInstantiator;
 import com.yahoo.elide.core.security.checks.Check;
@@ -24,6 +23,7 @@
 import com.yahoo.elide.core.security.permissions.expressions.Expression;
 import com.yahoo.elide.core.security.permissions.expressions.OrExpression;
 import com.yahoo.elide.core.security.permissions.expressions.SpecificFieldExpression;
+import com.yahoo.elide.core.security.visitors.PermissionExpressionNormalizationVisitor;
 import com.yahoo.elide.core.security.visitors.PermissionExpressionVisitor;
 import com.yahoo.elide.core.security.visitors.PermissionToFilterExpressionVisitor;
 import com.yahoo.elide.core.type.Type;
@@ -184,8 +184,8 @@ private Expression buildSpecificFieldExpression(final PermissionCondition condit
         ParseTree fieldPermissions = entityDictionary.getPermissionsForField(resourceClass, field, annotationClass);
 
         return new SpecificFieldExpression(condition,
-                expressionFromParseTree(classPermissions, checkFn),
-                expressionFromParseTree(fieldPermissions, checkFn)
+                normalizedExpressionFromParseTree(classPermissions, checkFn),
+                normalizedExpressionFromParseTree(fieldPermissions, checkFn)
         );
     }
 
@@ -205,7 +205,7 @@ private Expression buildAnyFieldExpression(final PermissionCondition condition,
         Class<? extends Annotation> annotationClass = condition.getPermission();
 
         ParseTree classPermissions = entityDictionary.getPermissionsForClass(resourceClass, annotationClass);
-        Expression entityExpression = expressionFromParseTree(classPermissions, checkFn);
+        Expression entityExpression = normalizedExpressionFromParseTree(classPermissions, checkFn);
 
         OrExpression allFieldsExpression = new OrExpression(FAILURE, null);
         List<String> fields = entityDictionary.getAllFields(resourceClass);
@@ -218,7 +218,7 @@ private Expression buildAnyFieldExpression(final PermissionCondition condition,
             }
 
             ParseTree fieldPermissions = entityDictionary.getPermissionsForField(resourceClass, field, annotationClass);
-            Expression fieldExpression = expressionFromParseTree(fieldPermissions, checkFn);
+            Expression fieldExpression = normalizedExpressionFromParseTree(fieldPermissions, checkFn);
 
             allFieldsExpression = new OrExpression(allFieldsExpression, fieldExpression);
         }
@@ -285,22 +285,25 @@ public FilterExpression buildAnyFieldFilterExpression(Type<?> forType, RequestSc
         return allFieldsFilterExpression;
     }
 
-    private Expression expressionFromParseTree(ParseTree permissions, Function<Check, Expression> checkFn) {
+    private Expression normalizedExpressionFromParseTree(ParseTree permissions, Function<Check, Expression> checkFn) {
         if (permissions == null) {
             return null;
         }
 
-        return new PermissionExpressionVisitor(entityDictionary, checkFn).visit(permissions);
+        return permissions
+                .accept(new PermissionExpressionVisitor(entityDictionary, checkFn))
+                .accept(new PermissionExpressionNormalizationVisitor());
     }
 
     private FilterExpression filterExpressionFromParseTree(ParseTree permissions, Type type, RequestScope scope) {
         if (permissions == null) {
             return null;
         }
+        final Function<Check, Expression> checkFn = (check) ->
+                new CheckExpression(check, null, scope, null, cache);
 
-        FilterExpression expression = new PermissionToFilterExpressionVisitor(entityDictionary, scope, type)
-                .visit(permissions);
-        return expression.accept(new FilterExpressionNormalizationVisitor());
+        return normalizedExpressionFromParseTree(permissions, checkFn)
+                .accept(new PermissionToFilterExpressionVisitor(entityDictionary, scope, type));
     }
 
     private Function<Check, Expression> leafBuilder(PersistentResource resource, ChangeSpec changeSpec) {

diff --git a/...re/src/main/java/com/yahoo/elide/core/security/permissions/expressions/AndExpression.java b/...re/src/main/java/com/yahoo/elide/core/security/permissions/expressions/AndExpression.java
@@ -10,11 +10,15 @@
 import static com.yahoo.elide.core.security.permissions.ExpressionResult.PASS;
 import com.yahoo.elide.core.security.permissions.ExpressionResult;
 
+import lombok.Getter;
+
 /**
  * Representation for an "And" expression.
  */
 public class AndExpression implements Expression {
+    @Getter
     private final Expression left;
+    @Getter
     private final Expression right;
 
     /**
@@ -50,6 +54,11 @@ public ExpressionResult evaluate(EvaluationMode mode) {
         return DEFERRED;
     }
 
+    @Override
+    public <T> T accept(ExpressionVisitor<T> visitor) {
+        return visitor.visitAndExpression(this);
+    }
+
     @Override
     public String toString() {
         if (right == null) {

diff --git a/...c/main/java/com/yahoo/elide/core/security/permissions/expressions/AnyFieldExpression.java b/...c/main/java/com/yahoo/elide/core/security/permissions/expressions/AnyFieldExpression.java
@@ -41,6 +41,11 @@ public ExpressionResult evaluate(EvaluationMode mode) {
         return (entityExpression == null) ? PASS : entityExpression.evaluate(mode);
     }
 
+    @Override
+    public <T> T accept(ExpressionVisitor<T> visitor) {
+        return visitor.visitExpression(this);
+    }
+
     @Override
     public String toString() {
         return String.format("%s FOR EXPRESSION [(FIELDS(%s)) OR (ENTITY(%s))]",

diff --git a/.../src/main/java/com/yahoo/elide/core/security/permissions/expressions/CheckExpression.java b/.../src/main/java/com/yahoo/elide/core/security/permissions/expressions/CheckExpression.java
@@ -18,6 +18,8 @@
 import com.yahoo.elide.core.security.checks.UserCheck;
 import com.yahoo.elide.core.security.permissions.ExpressionResult;
 import com.yahoo.elide.core.security.permissions.ExpressionResultCache;
+
+import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 
 import java.util.Optional;
@@ -28,6 +30,7 @@
 @Slf4j
 public class CheckExpression implements Expression {
 
+    @Getter
     protected final Check check;
     protected final PersistentResource resource;
     protected final RequestScope requestScope;
@@ -108,6 +111,11 @@ public ExpressionResult evaluate(EvaluationMode mode) {
         return result;
     }
 
+    @Override
+    public <T> T accept(ExpressionVisitor<T> visitor) {
+        return visitor.visitCheckExpression(this);
+    }
+
     /**
      * Actually compute the result of the check without caching concerns.
      *

diff --git a/...-core/src/main/java/com/yahoo/elide/core/security/permissions/expressions/Expression.java b/...-core/src/main/java/com/yahoo/elide/core/security/permissions/expressions/Expression.java
@@ -31,6 +31,8 @@ public enum EvaluationMode {
      */
     ExpressionResult evaluate(EvaluationMode mode);
 
+    public <T> T accept(ExpressionVisitor<T> visitor);
+
     /**
      * Static Expressions that return PASS or FAIL.
      */
@@ -41,6 +43,11 @@ public ExpressionResult evaluate(EvaluationMode ignored) {
                 return ExpressionResult.PASS;
             }
 
+            @Override
+            public <T> T accept(ExpressionVisitor<T> visitor) {
+                return visitor.visitExpression(this);
+            }
+
             @Override
             public String toString() {
                 return ansi().fg(Ansi.Color.GREEN).a("SUCCESS").reset().toString();
@@ -52,6 +59,11 @@ public ExpressionResult evaluate(EvaluationMode ignored) {
                 return ExpressionResult.FAIL;
             }
 
+            @Override
+            public <T> T accept(ExpressionVisitor<T> visitor) {
+                return visitor.visitExpression(this);
+            }
+
             @Override
             public String toString() {
                 return ansi().fg(Ansi.Color.RED).a("FAILURE").reset().toString();

diff --git a/...rc/main/java/com/yahoo/elide/core/security/permissions/expressions/ExpressionVisitor.java b/...rc/main/java/com/yahoo/elide/core/security/permissions/expressions/ExpressionVisitor.java
@@ -0,0 +1,19 @@
+/*
+ * Copyright 2021, Yahoo Inc.
+ * Licensed under the Apache License, Version 2.0
+ * See LICENSE file in project root for terms.
+ */
+
+package com.yahoo.elide.core.security.permissions.expressions;
+
+/**
+ * Visitor which walks the permission expression abstract syntax tree.
+ * @param <T> The return type of the visitor
+ */
+public interface ExpressionVisitor<T> {
+    T visitExpression(Expression expression);
+    T visitCheckExpression(CheckExpression checkExpression);
+    T visitAndExpression(AndExpression andExpression);
+    T visitOrExpression(OrExpression orExpression);
+    T visitNotExpression(NotExpression notExpression);
+}
diff --git a/...re/src/main/java/com/yahoo/elide/core/security/permissions/expressions/NotExpression.java b/...re/src/main/java/com/yahoo/elide/core/security/permissions/expressions/NotExpression.java
@@ -10,10 +10,13 @@
 import static com.yahoo.elide.core.security.permissions.ExpressionResult.PASS;
 import com.yahoo.elide.core.security.permissions.ExpressionResult;
 
+import lombok.Getter;
+
 /**
  * Representation of a "not" expression.
  */
 public class NotExpression implements Expression {
+    @Getter
     private final Expression logical;
 
     /**
@@ -41,6 +44,11 @@ public ExpressionResult evaluate(EvaluationMode mode) {
         return DEFERRED;
     }
 
+    @Override
+    public <T> T accept(ExpressionVisitor<T> visitor) {
+        return visitor.visitNotExpression(this);
+    }
+
     @Override
     public String toString() {
         return String.format("NOT (%s)", logical);

diff --git a/...ore/src/main/java/com/yahoo/elide/core/security/permissions/expressions/OrExpression.java b/...ore/src/main/java/com/yahoo/elide/core/security/permissions/expressions/OrExpression.java
@@ -10,11 +10,15 @@
 import static com.yahoo.elide.core.security.permissions.ExpressionResult.PASS;
 import com.yahoo.elide.core.security.permissions.ExpressionResult;
 
+import lombok.Getter;
+
 /**
  * Representation of an "or" expression.
  */
 public class OrExpression implements Expression {
+    @Getter
     private final Expression left;
+    @Getter
     private final Expression right;
 
     public static final OrExpression SUCCESSFUL_EXPRESSION = new OrExpression(Results.SUCCESS, null);
@@ -53,6 +57,11 @@ public ExpressionResult evaluate(EvaluationMode mode) {
         return DEFERRED;
     }
 
+    @Override
+    public <T> T accept(ExpressionVisitor<T> visitor) {
+        return visitor.visitOrExpression(this);
+    }
+
     @Override
     public String toString() {
         if (right == null || right.equals(Results.FAILURE)) {

diff --git a/...n/java/com/yahoo/elide/core/security/permissions/expressions/SpecificFieldExpression.java b/...n/java/com/yahoo/elide/core/security/permissions/expressions/SpecificFieldExpression.java
@@ -39,6 +39,11 @@ public ExpressionResult evaluate(EvaluationMode mode) {
         return fieldExpression.get().evaluate(mode);
     }
 
+    @Override
+    public <T> T accept(ExpressionVisitor<T> visitor) {
+        return visitor.visitExpression(this);
+    }
+
 
     @Override
     public String toString() {

diff --git a/...java/com/yahoo/elide/core/security/visitors/PermissionExpressionNormalizationVisitor.java b/...java/com/yahoo/elide/core/security/visitors/PermissionExpressionNormalizationVisitor.java
@@ -0,0 +1,65 @@
+/*
+ * Copyright 2021, Yahoo Inc.
+ * Licensed under the Apache License, Version 2.0
+ * See LICENSE file in project root for terms.
+ */
+
+package com.yahoo.elide.core.security.visitors;
+
+import com.yahoo.elide.core.security.permissions.expressions.AndExpression;
+import com.yahoo.elide.core.security.permissions.expressions.CheckExpression;
+import com.yahoo.elide.core.security.permissions.expressions.Expression;
+import com.yahoo.elide.core.security.permissions.expressions.ExpressionVisitor;
+import com.yahoo.elide.core.security.permissions.expressions.NotExpression;
+import com.yahoo.elide.core.security.permissions.expressions.OrExpression;
+
+/**
+ * Expression Visitor to normalize Permission expression.
+ */
+public class PermissionExpressionNormalizationVisitor implements ExpressionVisitor<Expression> {
+    @Override
+    public Expression visitExpression(Expression expression) {
+        return expression;
+    }
+
+    @Override
+    public Expression visitCheckExpression(CheckExpression checkExpression) {
+        return checkExpression;
+    }
+
+    @Override
+    public Expression visitAndExpression(AndExpression andExpression) {
+        Expression left = andExpression.getLeft();
+        Expression right = andExpression.getRight();
+        return new AndExpression(left.accept(this), right.accept(this));
+    }
+
+    @Override
+    public Expression visitOrExpression(OrExpression orExpression) {
+        Expression left = orExpression.getLeft();
+        Expression right = orExpression.getRight();
+        return new OrExpression(left.accept(this), right.accept(this));
+    }
+
+    @Override
+    public Expression visitNotExpression(NotExpression notExpression) {
+        Expression inner = notExpression.getLogical();
+        if (inner instanceof AndExpression) {
+            AndExpression and = (AndExpression) inner;
+            Expression left = new NotExpression(and.getLeft()).accept(this);
+            Expression right = new NotExpression(and.getRight()).accept(this);
+            return new OrExpression(left, right);
+        }
+        if (inner instanceof OrExpression) {
+            OrExpression or = (OrExpression) inner;
+            Expression left = new NotExpression(or.getLeft()).accept(this);
+            Expression right = new NotExpression(or.getRight()).accept(this);
+            return new AndExpression(left, right);
+        }
+        if (inner instanceof NotExpression) {
+            NotExpression not = (NotExpression) inner;
+            return (not.getLogical()).accept(this);
+        }
+        return notExpression;
+    }
+}