Elide 5.0 - Simplify Security Checks

[aklish]

This issue should encompass the following improvements:

1. We should eliminate the notion of inline and commit checks. The Elide framework should run all checks inline except for newly created objects (which will be run at commit).
2. Simply to three kinds of checks
   • Checks which return true/false based on a User object (equivalent of UserCheck)
   • Checks which return true/false based on the entity being read/written plus an optional Change Spec)
   • Checks which return a predicate which can be pushed to the datastore or evaluated in memory.
3. Check functions should not take a RequestScope. Instead they should take a User.
4. Checks should be injectable (similar to entity beans) so they can have access to other services if needed.

[QubitPi]

Implementing #753

The requirements of #753 are best satisfied by having Check as the top interface type:

@FunctionalInterface
public interface Check {

    default String checkIdentifier() {
        return this.getClass().getName();
    }
}

and have 3 sub-types as the aforementioned "three kinds of checks":

// Checks which return true/false based on a User object (equivalent of UserCheck)
public interface UserCheck extends Check {
    
    boolean ok(User user);
}

// Checks which return true/false based on the entity being read/written plus an optional Change Spec)
public interface OperationCheck<T> extends Check {
    
    default boolean ok(T object, User user) {
        return ok(object, user, null);
    }

    boolean ok(T object, User user, ChangeSpec changeSpec);
}

// Checks which return a predicate which can be pushed to the datastore or evaluated in memory.
public interface FilterExpressionCheck<T> extends OperationCheck<T> {

    FilterExpression getFilterExpression(Class<?> entityClass, RequestScope requestScope);
}

public abstract class AbstractFilterExpressionCheck<T> implements FilterExpressionCheck<T> {

    @Override
    public final boolean ok(T object, RequestScope requestScope, ChangeSpec changeSpec) {
        Class entityClass = coreScope(requestScope).getDictionary().lookupEntityClass(object.getClass());
        FilterExpression filterExpression = getFilterExpression(entityClass, requestScope);
        return filterExpression.accept(new FilterExpressionCheckEvaluationVisitor(object, this, requestScope));
    }
    
    public boolean applyPredicateToObject(T object, FilterPredicate filterPredicate, RequestScope requestScope) {
        try {
            String fieldPath = filterPredicate.getFieldPath();
            com.yahoo.elide.core.RequestScope scope = coreScope(requestScope);
            Predicate fn = filterPredicate.getOperator().contextualize(fieldPath, filterPredicate.getValues(), scope);
            return fn.test(object);
        } catch (Exception e) {
            log.error("Failed to apply predicate {}", filterPredicate, e);
            return false;
        }
    }
    
    protected static Path getFieldPath(Class<?> type, RequestScope requestScope, String method, String defaultPath) {
        EntityDictionary dictionary = coreScope(requestScope).getDictionary();
        try {
            FilterExpressionPath fep = getFilterExpressionPath(type, method, dictionary);
            return new Path(type, dictionary, fep == null ? defaultPath : fep.value());
        } catch (NoSuchMethodException | SecurityException e) {
            throw new IllegalStateException(e);
        }
    }

    private static FilterExpressionPath getFilterExpressionPath(
            Class<?> type,
            String method,
            EntityDictionary dictionary) throws NoSuchMethodException {
        FilterExpressionPath path = dictionary.lookupEntityClass(type)
                .getMethod(method)
                .getAnnotation(FilterExpressionPath.class);
        return path;
    }

    protected static com.yahoo.elide.core.RequestScope coreScope(RequestScope requestScope) {
        return (com.yahoo.elide.core.RequestScope) requestScope;
    }
}

It is also suggested to add a reference to RequestScope to User so that all checks only need to "take a user".

With the proposal above, the security framework becomes simpler because instead of intermingle functionalities between different interface types, we now separate them by grouping different checks to sub-types, each contracting a specific check(user, operation, filter expression).

Implementations can definitely be made injectable and "newly created objects" can implement any one of the 3 sub-interfaces that are not inline.

[aklish]

Looks good, but the FilterExpressionCheck looks identical to the code today. Was that intentional or did you plan on making some edits there?

A few caveats:

• One thing I like about the existing FilterExpressionCheck is that it is not possible to override the check functions. They are final. This is important because Elide depends on the ability to execute them in-memory like a standard OperationCheck. I"m not sure we should also have an interface because someone might subclass that directly. I'd rather lock down the checks so there is only one way to use them.

[QubitPi]

Make sense. I see your concern. Let me spend some time figuring out a more rigorous design on Friday.

[QubitPi]

@aklish In order to make the operation check final, looks like we want a concrete check that cannot have variant. How about making FilterExpressionCheck a concrete class then and marking the check method as final?

public class FilterExpressionCheck<T> implements OperationCheck<T> {

    @Getter
    private final FilterExpression filterExpression;
    
    public FilterExpressionCheck(FilterExpression filterExpression) {
        this.filterExpression = Objects.requireNonNull(filterExpression, "filterExpression");
    }

    @Override
    public final boolean ok(T object, RequestScope requestScope, ChangeSpec changeSpec) {
        Class entityClass = coreScope(requestScope).getDictionary().lookupEntityClass(object.getClass());
        FilterExpression filterExpression = getFilterExpression(entityClass, requestScope);
        return filterExpression.accept(new FilterExpressionCheckEvaluationVisitor(object, this, requestScope));
    }
    
    public boolean applyPredicateToObject(T object, FilterPredicate filterPredicate, RequestScope requestScope) {
        try {
            String fieldPath = filterPredicate.getFieldPath();
            com.yahoo.elide.core.RequestScope scope = coreScope(requestScope);
            Predicate fn = filterPredicate.getOperator().contextualize(fieldPath, filterPredicate.getValues(), scope);
            return fn.test(object);
        } catch (Exception e) {
            log.error("Failed to apply predicate {}", filterPredicate, e);
            return false;
        }
    }
    
    protected static Path getFieldPath(Class<?> type, RequestScope requestScope, String method, String defaultPath) {
        EntityDictionary dictionary = coreScope(requestScope).getDictionary();
        try {
            FilterExpressionPath fep = getFilterExpressionPath(type, method, dictionary);
            return new Path(type, dictionary, fep == null ? defaultPath : fep.value());
        } catch (NoSuchMethodException | SecurityException e) {
            throw new IllegalStateException(e);
        }
    }

    private static FilterExpressionPath getFilterExpressionPath(
            Class<?> type,
            String method,
            EntityDictionary dictionary) throws NoSuchMethodException {
        FilterExpressionPath path = dictionary.lookupEntityClass(type)
                .getMethod(method)
                .getAnnotation(FilterExpressionPath.class);
        return path;
    }

    protected static com.yahoo.elide.core.RequestScope coreScope(RequestScope requestScope) {
        return (com.yahoo.elide.core.RequestScope) requestScope;
    }
}

The upside of this is that

1. we simply things essentially to 2 types of checks, in stead of 3
2. In addition, we make the check final
3. Also it makes sense to see FilterExpressionCheck as a kind of OperationCheck