diff --git a/elide-core/src/main/java/com/yahoo/elide/core/VerifyFieldAccessFilterExpressionVisitor.java b/elide-core/src/main/java/com/yahoo/elide/core/VerifyFieldAccessFilterExpressionVisitor.java index 8fa2ffb7e3..08fac9e4da 100644 --- a/elide-core/src/main/java/com/yahoo/elide/core/VerifyFieldAccessFilterExpressionVisitor.java +++ b/elide-core/src/main/java/com/yahoo/elide/core/VerifyFieldAccessFilterExpressionVisitor.java @@ -13,6 +13,8 @@ import com.yahoo.elide.core.filter.expression.FilterExpressionVisitor; import com.yahoo.elide.core.filter.expression.NotFilterExpression; import com.yahoo.elide.core.filter.expression.OrFilterExpression; +import com.yahoo.elide.security.PermissionExecutor; +import com.yahoo.elide.security.permissions.ExpressionResult; import java.util.Collections; import java.util.Objects; @@ -40,6 +42,15 @@ public VerifyFieldAccessFilterExpressionVisitor(PersistentResource resource) public Boolean visitPredicate(FilterPredicate filterPredicate) { RequestScope requestScope = resource.getRequestScope(); Set val = Collections.singleton(resource); + + ExpressionResult result = evaluateUserChecks(filterPredicate.getPath()); + + if (result == ExpressionResult.PASS) { + return true; + } else if (result == ExpressionResult.FAIL) { + return false; + } + for (Path.PathElement pathElement : filterPredicate.getPath().getPathElements()) { String fieldName = pathElement.getFieldName(); @@ -74,6 +85,28 @@ private Stream getValueChecked(PersistentResource resourc return resource.getRelationChecked(fieldName, Optional.empty(), Optional.empty(), Optional.empty()).stream(); } + private ExpressionResult evaluateUserChecks(Path path) { + PermissionExecutor executor = resource.getRequestScope().getPermissionExecutor(); + + ExpressionResult result = ExpressionResult.PASS; + + for (Path.PathElement element : path.getPathElements()) { + try { + result = executor.checkUserPermissions( + element.getType(), + ReadPermission.class, + element.getFieldName()); + } catch (ForbiddenAccessException e) { + return ExpressionResult.FAIL; + } + + if (result != ExpressionResult.PASS) { + return result; + } + } + return result; + } + @Override public Boolean visitAndExpression(AndFilterExpression expression) { FilterExpression left = expression.getLeft(); diff --git a/elide-core/src/main/java/com/yahoo/elide/security/PermissionExecutor.java b/elide-core/src/main/java/com/yahoo/elide/security/PermissionExecutor.java index 45e927d3e1..d1f1673c58 100644 --- a/elide-core/src/main/java/com/yahoo/elide/security/PermissionExecutor.java +++ b/elide-core/src/main/java/com/yahoo/elide/security/PermissionExecutor.java @@ -88,6 +88,17 @@ ExpressionResult checkSpecificFieldPermissionsDeferred(Pe */ ExpressionResult checkUserPermissions(Class resourceClass, Class annotationClass); + /** + * Check strictly user permissions on an entity field. + * + * @param type parameter + * @param resourceClass Resource class + * @param annotationClass Annotation class + * @param field The entity field + */ + public ExpressionResult checkUserPermissions(Class resourceClass, + Class annotationClass, + String field); /** * Get the read filter, if defined. * diff --git a/elide-core/src/main/java/com/yahoo/elide/security/executors/ActivePermissionExecutor.java b/elide-core/src/main/java/com/yahoo/elide/security/executors/ActivePermissionExecutor.java index 607718aa37..7d1f3a3916 100644 --- a/elide-core/src/main/java/com/yahoo/elide/security/executors/ActivePermissionExecutor.java +++ b/elide-core/src/main/java/com/yahoo/elide/security/executors/ActivePermissionExecutor.java @@ -491,4 +491,31 @@ public String printCheckStats() { public boolean isVerbose() { return verbose; } + + /** + * Check strictly user permissions on an entity field. + * + * @param type parameter + * @param resourceClass Resource class + * @param annotationClass Annotation class + * @param field The entity field + */ + @Override + public ExpressionResult checkUserPermissions(Class resourceClass, + Class annotationClass, + String field) { + Supplier expressionSupplier = () -> { + return expressionBuilder.buildUserCheckFieldExpressions( + resourceClass, + requestScope, + annotationClass, + field); + }; + + return checkOnlyUserPermissions( + resourceClass, + annotationClass, + Optional.of(field), + expressionSupplier); + } } diff --git a/elide-core/src/main/java/com/yahoo/elide/security/executors/BypassPermissionExecutor.java b/elide-core/src/main/java/com/yahoo/elide/security/executors/BypassPermissionExecutor.java index 5306b164d5..f66d1b76b0 100644 --- a/elide-core/src/main/java/com/yahoo/elide/security/executors/BypassPermissionExecutor.java +++ b/elide-core/src/main/java/com/yahoo/elide/security/executors/BypassPermissionExecutor.java @@ -54,6 +54,13 @@ public Optional getReadPermissionFilter(Class resourceClass return Optional.empty(); } + @Override + public ExpressionResult checkUserPermissions(Class resourceClass, + Class annotationClass, + String field) { + return ExpressionResult.PASS; + } + @Override public void executeCommitChecks() { diff --git a/elide-core/src/main/java/com/yahoo/elide/security/permissions/PermissionCondition.java b/elide-core/src/main/java/com/yahoo/elide/security/permissions/PermissionCondition.java index b493143d96..35602b4c7e 100644 --- a/elide-core/src/main/java/com/yahoo/elide/security/permissions/PermissionCondition.java +++ b/elide-core/src/main/java/com/yahoo/elide/security/permissions/PermissionCondition.java @@ -97,6 +97,14 @@ public static PermissionCondition create( this.field = Optional.empty(); } + PermissionCondition(Class permission, Class entityClass, String field) { + this.permission = permission; + this.resource = Optional.empty(); + this.entityClass = entityClass; + this.changes = Optional.empty(); + this.field = Optional.of(field); + } + PermissionCondition(Class permission, PersistentResource resource, String field) { this.permission = permission; this.resource = Optional.of(resource); diff --git a/elide-core/src/main/java/com/yahoo/elide/security/permissions/PermissionExpressionBuilder.java b/elide-core/src/main/java/com/yahoo/elide/security/permissions/PermissionExpressionBuilder.java index 972b92451e..428298c9f6 100644 --- a/elide-core/src/main/java/com/yahoo/elide/security/permissions/PermissionExpressionBuilder.java +++ b/elide-core/src/main/java/com/yahoo/elide/security/permissions/PermissionExpressionBuilder.java @@ -124,22 +124,26 @@ public Expression buildAnyFieldExpressions(final Persiste *

* NOTE: This method returns _NO_ commit checks. * - * @param resource Resource + * @param resourceClass Resource Class + * @param scope The request scope. * @param annotationClass Annotation class * @param field Field to check (if null only check entity-level) * @param type parameter * @return User check expression to evaluate */ - public Expression buildUserCheckFieldExpressions(final PersistentResource resource, + public Expression buildUserCheckFieldExpressions(final Class resourceClass, + final RequestScope scope, final Class annotationClass, final String field) { - Class resourceClass = resource.getResourceClass(); if (!entityDictionary.entityHasChecksForPermission(resourceClass, annotationClass)) { return SUCCESSFUL_EXPRESSION; } - final Function leafBuilderFn = leafBuilder(resource, null); - return buildSpecificFieldExpression(new PermissionCondition(annotationClass, resource, field), leafBuilderFn); + final Function leafBuilderFn = (check) -> + new CheckExpression(check, null, scope, null, cache); + + return buildSpecificFieldExpression(new PermissionCondition(annotationClass, resourceClass, field), + leafBuilderFn); } /** diff --git a/elide-core/src/test/java/com/yahoo/elide/core/VerifyFieldAccessFilterExpressionVisitorTest.java b/elide-core/src/test/java/com/yahoo/elide/core/VerifyFieldAccessFilterExpressionVisitorTest.java index 060c272d3d..647c4623a2 100644 --- a/elide-core/src/test/java/com/yahoo/elide/core/VerifyFieldAccessFilterExpressionVisitorTest.java +++ b/elide-core/src/test/java/com/yahoo/elide/core/VerifyFieldAccessFilterExpressionVisitorTest.java @@ -7,7 +7,9 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -16,11 +18,14 @@ import com.yahoo.elide.core.exceptions.ForbiddenAccessException; import com.yahoo.elide.core.filter.FilterPredicate; import com.yahoo.elide.core.filter.InPredicate; +import com.yahoo.elide.core.filter.dialect.RSQLFilterDialect; import com.yahoo.elide.core.filter.expression.AndFilterExpression; +import com.yahoo.elide.core.filter.expression.FilterExpression; import com.yahoo.elide.core.filter.expression.NotFilterExpression; import com.yahoo.elide.core.filter.expression.OrFilterExpression; import com.yahoo.elide.security.PermissionExecutor; +import com.yahoo.elide.security.permissions.ExpressionResult; import example.Author; import example.Book; @@ -158,4 +163,77 @@ public void testReject() throws Exception { verify(permissionExecutor, times(5)).checkSpecificFieldPermissions(resource, null, ReadPermission.class, HOME); } + + @Test + public void testShortCircuitReject() throws Exception { + RSQLFilterDialect dialect = new RSQLFilterDialect(scope.getDictionary()); + FilterExpression expression = + dialect.parseFilterExpression("genre==foo", Book.class, true); + + Book book = new Book(); + PersistentResource resource = new PersistentResource<>(book, null, "", scope); + + PermissionExecutor permissionExecutor = scope.getPermissionExecutor(); + + when(permissionExecutor.checkUserPermissions(Book.class, ReadPermission.class, GENRE)) + .thenThrow(ForbiddenAccessException.class); + + VerifyFieldAccessFilterExpressionVisitor visitor = new VerifyFieldAccessFilterExpressionVisitor(resource); + // restricted HOME field + assertFalse(expression.accept(visitor)); + + verify(permissionExecutor, times(1)).checkUserPermissions(Book.class, ReadPermission.class, GENRE); + verify(permissionExecutor, never()).checkSpecificFieldPermissions(resource, null, ReadPermission.class, GENRE); + } + + @Test + public void testShortCircuitDeferred() throws Exception { + RSQLFilterDialect dialect = new RSQLFilterDialect(scope.getDictionary()); + FilterExpression expression = + dialect.parseFilterExpression("genre==foo", Book.class, true); + + Book book = new Book(); + PersistentResource resource = new PersistentResource<>(book, null, "", scope); + + PermissionExecutor permissionExecutor = scope.getPermissionExecutor(); + + when(permissionExecutor.checkUserPermissions(Book.class, ReadPermission.class, GENRE)) + .thenReturn(ExpressionResult.DEFERRED); + when(permissionExecutor.checkSpecificFieldPermissions(resource, null, ReadPermission.class, GENRE)) + .thenThrow(ForbiddenAccessException.class); + + VerifyFieldAccessFilterExpressionVisitor visitor = new VerifyFieldAccessFilterExpressionVisitor(resource); + // restricted HOME field + assertFalse(expression.accept(visitor)); + + verify(permissionExecutor, times(1)).checkUserPermissions(Book.class, ReadPermission.class, GENRE); + verify(permissionExecutor, times(1)).checkSpecificFieldPermissions(resource, null, ReadPermission.class, GENRE); + } + + @Test + public void testShortCircuitPass() throws Exception { + RSQLFilterDialect dialect = new RSQLFilterDialect(scope.getDictionary()); + FilterExpression expression = + dialect.parseFilterExpression("authors.name==foo", Book.class, true); + + Book book = new Book(); + PersistentResource resource = new PersistentResource<>(book, null, "", scope); + + PermissionExecutor permissionExecutor = scope.getPermissionExecutor(); + DataStoreTransaction tx = scope.getTransaction(); + + when(permissionExecutor.checkUserPermissions(Book.class, ReadPermission.class, "authors")) + .thenReturn(ExpressionResult.PASS); + when(permissionExecutor.checkUserPermissions(Author.class, ReadPermission.class, NAME)) + .thenReturn(ExpressionResult.PASS); + + VerifyFieldAccessFilterExpressionVisitor visitor = new VerifyFieldAccessFilterExpressionVisitor(resource); + // restricted HOME field + assertTrue(expression.accept(visitor)); + + verify(permissionExecutor, times(1)).checkUserPermissions(Book.class, ReadPermission.class, "authors"); + verify(permissionExecutor, times(1)).checkUserPermissions(Author.class, ReadPermission.class, "name"); + verify(permissionExecutor, never()).checkSpecificFieldPermissions(resource, null, ReadPermission.class, GENRE); + verify(tx, never()).getRelation(any(), any(), any(), any(), any(), any(), any()); + } } diff --git a/elide-core/src/test/java/com/yahoo/elide/security/PermissionExecutorTest.java b/elide-core/src/test/java/com/yahoo/elide/security/PermissionExecutorTest.java index db5c42da02..9777d095c8 100644 --- a/elide-core/src/test/java/com/yahoo/elide/security/PermissionExecutorTest.java +++ b/elide-core/src/test/java/com/yahoo/elide/security/PermissionExecutorTest.java @@ -5,6 +5,7 @@ */ package com.yahoo.elide.security; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import com.yahoo.elide.ElideSettings; @@ -21,6 +22,7 @@ import com.yahoo.elide.security.checks.OperationCheck; import com.yahoo.elide.security.checks.UserCheck; +import com.yahoo.elide.security.permissions.ExpressionResult; import example.TestCheckMappings; import org.junit.jupiter.api.Test; @@ -413,6 +415,40 @@ public void testUserCheckCache() { requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource, cspec); } + @Test + public void testUserCheckOnFieldSuccess() { + PersistentResource resource = newResource(OpenBean.class); + RequestScope requestScope = resource.getRequestScope(); + ExpressionResult result = requestScope.getPermissionExecutor().checkUserPermissions(OpenBean.class, + ReadPermission.class, + "open"); + + assertEquals(ExpressionResult.PASS, result); + } + + @Test + public void testUserCheckOnFieldFailure() { + PersistentResource resource = newResource(SampleBean.class); + RequestScope requestScope = resource.getRequestScope(); + assertThrows( + ForbiddenAccessException.class, + () -> requestScope.getPermissionExecutor().checkUserPermissions(SampleBean.class, + ReadPermission.class, + "cannotSeeMe")); + } + + @Test + public void testUserCheckOnFieldDeferred() { + PersistentResource resource = newResource(SampleBean.class); + RequestScope requestScope = resource.getRequestScope(); + + ExpressionResult result = requestScope.getPermissionExecutor().checkUserPermissions(SampleBean.class, + ReadPermission.class, + "allVisible"); + + assertEquals(ExpressionResult.DEFERRED, result); + } + public PersistentResource newResource(T obj, Class cls) { EntityDictionary dictionary = new EntityDictionary(TestCheckMappings.MAPPINGS); dictionary.bindEntity(cls);