diff --git a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/Check.java b/elide-annotations/src/main/java/com/yahoo/elide/security/checks/Check.java index 6a18e17cb4..d2fd64d6c7 100644 --- a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/Check.java +++ b/elide-annotations/src/main/java/com/yahoo/elide/security/checks/Check.java @@ -4,39 +4,9 @@ * See LICENSE file in project root for terms. */ package com.yahoo.elide.security.checks; - -import com.yahoo.elide.security.ChangeSpec; -import com.yahoo.elide.security.RequestScope; -import com.yahoo.elide.security.User; - -import java.util.Optional; - /** * Custom security access that verifies whether a user belongs to a role. * Permissions are assigned as a set of checks that grant access to the permission. - * @param Type of record for Check */ -public interface Check { - - /** - * Determines whether the user can access the resource. - * - * @param object Fully modified object - * @param requestScope Request scope object - * @param changeSpec Summary of modifications - * @return true if security check passed - */ - boolean ok(T object, RequestScope requestScope, Optional changeSpec); - - /** - * Method reserved for user checks. - * - * @param user User to check - * @return True if user check passes, false otherwise - */ - boolean ok(User user); - - default String checkIdentifier() { - return this.getClass().getName(); - } +public interface Check { } diff --git a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/CommitCheck.java b/elide-annotations/src/main/java/com/yahoo/elide/security/checks/CommitCheck.java deleted file mode 100644 index e62d3909aa..0000000000 --- a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/CommitCheck.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright 2016, Yahoo Inc. - * Licensed under the Apache License, Version 2.0 - * See LICENSE file in project root for terms. - */ -package com.yahoo.elide.security.checks; - -import com.yahoo.elide.security.User; - -/** - * Commit check interface. - * @see Check - * - * Commit checks are run immediately before a transaction is about to commit but after all changes have been made. - * Objects passed to this check are guaranteed to be in their final state. - * - * @param Type parameter - */ -public abstract class CommitCheck implements Check { - /* NOTE: Operation checks and user checks are intended to be _distinct_ */ - @Override - public final boolean ok(User user) { - throw new UnsupportedOperationException(); - } -} diff --git a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/InlineCheck.java b/elide-annotations/src/main/java/com/yahoo/elide/security/checks/InlineCheck.java deleted file mode 100644 index 8f9c38cb3a..0000000000 --- a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/InlineCheck.java +++ /dev/null @@ -1,16 +0,0 @@ -/* - * Copyright 2016, Yahoo Inc. - * Licensed under the Apache License, Version 2.0 - * See LICENSE file in project root for terms. - */ -package com.yahoo.elide.security.checks; - -/** - * Intermediate check representing the hierarchical structure of checks. - * For instance, Read/Delete permissions can take any type of InlineCheck - * while Create/Update permissions can be of any Check type. - * - * @param type parameter - */ -public abstract class InlineCheck implements Check { -} diff --git a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/OperationCheck.java b/elide-annotations/src/main/java/com/yahoo/elide/security/checks/OperationCheck.java index d02472239b..8b72185f13 100644 --- a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/OperationCheck.java +++ b/elide-annotations/src/main/java/com/yahoo/elide/security/checks/OperationCheck.java @@ -5,7 +5,10 @@ */ package com.yahoo.elide.security.checks; -import com.yahoo.elide.security.User; +import com.yahoo.elide.security.ChangeSpec; +import com.yahoo.elide.security.RequestScope; + +import java.util.Optional; /** * Operation check interface. @@ -18,10 +21,14 @@ * * @param Type parameter */ -public abstract class OperationCheck extends InlineCheck { - /* NOTE: Operation checks and user checks are intended to be _distinct_ */ - @Override - public final boolean ok(User user) { - throw new UnsupportedOperationException(); - } +public abstract class OperationCheck implements Check { + /** + * Determines whether the user can access the resource. + * + * @param object Fully modified object + * @param requestScope Request scope object + * @param changeSpec Summary of modifications + * @return true if security check passed + */ + public abstract boolean ok(T object, RequestScope requestScope, Optional changeSpec); } diff --git a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/UserCheck.java b/elide-annotations/src/main/java/com/yahoo/elide/security/checks/UserCheck.java index f44a953060..41e62c68bc 100644 --- a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/UserCheck.java +++ b/elide-annotations/src/main/java/com/yahoo/elide/security/checks/UserCheck.java @@ -5,19 +5,18 @@ */ package com.yahoo.elide.security.checks; -import com.yahoo.elide.security.ChangeSpec; -import com.yahoo.elide.security.RequestScope; - -import java.util.Optional; +import com.yahoo.elide.security.User; /** * Custom security access that verifies whether a user belongs to a role. * Permissions are assigned as a set of checks that grant access to the permission. */ -public abstract class UserCheck extends InlineCheck { - /* NOTE: Operation checks and user checks are intended to be _distinct_ */ - @Override - public final boolean ok(Object object, RequestScope requestScope, Optional changeSpec) { - return ok(requestScope.getUser()); - } +public abstract class UserCheck implements Check { + /** + * Method reserved for user checks. + * + * @param user User to check + * @return True if user check passes, false otherwise + */ + public abstract boolean ok(User user); } diff --git a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/prefab/Collections.java b/elide-annotations/src/main/java/com/yahoo/elide/security/checks/prefab/Collections.java index e91bfc77a9..7a21a3668b 100644 --- a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/prefab/Collections.java +++ b/elide-annotations/src/main/java/com/yahoo/elide/security/checks/prefab/Collections.java @@ -8,7 +8,7 @@ import com.yahoo.elide.security.ChangeSpec; import com.yahoo.elide.security.RequestScope; -import com.yahoo.elide.security.checks.CommitCheck; +import com.yahoo.elide.security.checks.OperationCheck; import java.util.Collection; import java.util.Optional; @@ -27,7 +27,7 @@ private Collections() { * * @param type collection to be validated */ - public static class AppendOnly extends CommitCheck { + public static class AppendOnly extends OperationCheck { @Override public boolean ok(T record, RequestScope requestScope, Optional changeSpec) { @@ -47,7 +47,7 @@ public boolean ok(T record, RequestScope requestScope, Optional chan * * @param type parameter */ - public static class RemoveOnly extends CommitCheck { + public static class RemoveOnly extends OperationCheck { @Override public boolean ok(T record, RequestScope requestScope, Optional changeSpec) { diff --git a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/prefab/Common.java b/elide-annotations/src/main/java/com/yahoo/elide/security/checks/prefab/Common.java index cd6600983c..1b75722215 100644 --- a/elide-annotations/src/main/java/com/yahoo/elide/security/checks/prefab/Common.java +++ b/elide-annotations/src/main/java/com/yahoo/elide/security/checks/prefab/Common.java @@ -9,7 +9,6 @@ import com.yahoo.elide.security.ChangeSpec; import com.yahoo.elide.security.PersistentResource; import com.yahoo.elide.security.RequestScope; -import com.yahoo.elide.security.checks.CommitCheck; import com.yahoo.elide.security.checks.OperationCheck; import java.util.Optional; @@ -42,7 +41,7 @@ public boolean ok(T record, RequestScope requestScope, Optional chan * but at the same time allows the removal of the child from the relationship with the existing parent * @param the type of object that this check guards */ - public static class FieldSetToNull extends CommitCheck { + public static class FieldSetToNull extends OperationCheck { @Override public boolean ok(T record, RequestScope requestScope, Optional changeSpec) { return changeSpec.map((c) -> { return c.getModified() == null; }) diff --git a/elide-core/src/main/java/com/yahoo/elide/parsers/expression/CanPaginateVisitor.java b/elide-core/src/main/java/com/yahoo/elide/parsers/expression/CanPaginateVisitor.java index 4cc0615419..449abf86bd 100644 --- a/elide-core/src/main/java/com/yahoo/elide/parsers/expression/CanPaginateVisitor.java +++ b/elide-core/src/main/java/com/yahoo/elide/parsers/expression/CanPaginateVisitor.java @@ -208,14 +208,14 @@ public PaginationStatus visitPAREN(ExpressionParser.PARENContext ctx) { @Override public PaginationStatus visitPermissionClass(ExpressionParser.PermissionClassContext ctx) { - Check check = getCheck(dictionary, ctx.getText()); + Check check = getCheck(dictionary, ctx.getText()); //Filter expression checks can always be pushed to the DataStore so pagination is possible if (check instanceof FilterExpressionCheck) { return PaginationStatus.CAN_PAGINATE; } if (check instanceof UserCheck) { - if (check.ok(scope.getUser())) { + if (((UserCheck) check).ok(scope.getUser())) { return PaginationStatus.USER_CHECK_TRUE; } return PaginationStatus.USER_CHECK_FALSE; diff --git a/elide-core/src/main/java/com/yahoo/elide/parsers/expression/PermissionToFilterExpressionVisitor.java b/elide-core/src/main/java/com/yahoo/elide/parsers/expression/PermissionToFilterExpressionVisitor.java index a8622f1bbc..5413b8cd35 100644 --- a/elide-core/src/main/java/com/yahoo/elide/parsers/expression/PermissionToFilterExpressionVisitor.java +++ b/elide-core/src/main/java/com/yahoo/elide/parsers/expression/PermissionToFilterExpressionVisitor.java @@ -177,7 +177,7 @@ public FilterExpression visitPermissionClass(ExpressionParser.PermissionClassCon } if (check instanceof UserCheck) { - boolean userCheckResult = check.ok(requestScope.getUser()); + boolean userCheckResult = ((UserCheck) check).ok(requestScope.getUser()); return userCheckResult ? TRUE_USER_CHECK_EXPRESSION : FALSE_USER_CHECK_EXPRESSION; } diff --git a/elide-core/src/main/java/com/yahoo/elide/security/FilterExpressionCheck.java b/elide-core/src/main/java/com/yahoo/elide/security/FilterExpressionCheck.java index 6a03492201..dff681d709 100644 --- a/elide-core/src/main/java/com/yahoo/elide/security/FilterExpressionCheck.java +++ b/elide-core/src/main/java/com/yahoo/elide/security/FilterExpressionCheck.java @@ -12,8 +12,8 @@ import com.yahoo.elide.core.filter.FilterPredicate; import com.yahoo.elide.core.filter.expression.FilterExpression; import com.yahoo.elide.parsers.expression.FilterExpressionCheckEvaluationVisitor; -import com.yahoo.elide.security.checks.InlineCheck; +import com.yahoo.elide.security.checks.OperationCheck; import lombok.extern.slf4j.Slf4j; import java.util.Optional; @@ -27,7 +27,7 @@ * @param Type of class */ @Slf4j -public abstract class FilterExpressionCheck extends InlineCheck { +public abstract class FilterExpressionCheck extends OperationCheck { @Inject protected EntityDictionary dictionary; @@ -41,12 +41,6 @@ public abstract class FilterExpressionCheck extends InlineCheck { */ public abstract FilterExpression getFilterExpression(Class entityClass, RequestScope requestScope); - /* NOTE: Filter Expression checks and user checks are intended to be _distinct_ */ - @Override - public final boolean ok(User user) { - throw new UnsupportedOperationException(); - } - /** * The filter expression is evaluated in memory if it cannot be pushed to the data store by elide for any reason. * diff --git a/elide-core/src/main/java/com/yahoo/elide/security/permissions/expressions/CheckExpression.java b/elide-core/src/main/java/com/yahoo/elide/security/permissions/expressions/CheckExpression.java index 2593a50458..36174d8232 100644 --- a/elide-core/src/main/java/com/yahoo/elide/security/permissions/expressions/CheckExpression.java +++ b/elide-core/src/main/java/com/yahoo/elide/security/permissions/expressions/CheckExpression.java @@ -15,7 +15,7 @@ import com.yahoo.elide.security.PersistentResource; import com.yahoo.elide.security.RequestScope; import com.yahoo.elide.security.checks.Check; -import com.yahoo.elide.security.checks.InlineCheck; +import com.yahoo.elide.security.checks.OperationCheck; import com.yahoo.elide.security.checks.UserCheck; import com.yahoo.elide.security.permissions.ExpressionResult; import com.yahoo.elide.security.permissions.ExpressionResultCache; @@ -76,7 +76,7 @@ public ExpressionResult evaluate(EvaluationMode mode) { return result; } - if (mode == EvaluationMode.INLINE_CHECKS_ONLY && ! (check instanceof InlineCheck)) { + if (mode == EvaluationMode.INLINE_CHECKS_ONLY && (resource != null && resource.isNewlyCreated())) { result = DEFERRED; return result; } @@ -112,7 +112,12 @@ public ExpressionResult evaluate(EvaluationMode mode) { */ private ExpressionResult computeCheck() { Object entity = (resource == null) ? null : resource.getObject(); - result = check.ok(entity, requestScope, changeSpec) ? PASS : FAIL; + + if (check instanceof UserCheck) { + result = ((UserCheck) check).ok(requestScope.getUser()) ? PASS : FAIL; + } else { + result = ((OperationCheck) check).ok(entity, requestScope, changeSpec) ? PASS : FAIL; + } return result; } diff --git a/elide-core/src/test/java/com/yahoo/elide/core/utils/ClassScannerTest.java b/elide-core/src/test/java/com/yahoo/elide/core/utils/ClassScannerTest.java index d800ce3fa3..1fe0571de5 100644 --- a/elide-core/src/test/java/com/yahoo/elide/core/utils/ClassScannerTest.java +++ b/elide-core/src/test/java/com/yahoo/elide/core/utils/ClassScannerTest.java @@ -44,7 +44,7 @@ public void testGetAllAnnotatedClasses() { @Test public void testGetAnyAnnotatedClasses() { Set> classes = ClassScanner.getAnnotatedClasses(ReadPermission.class, UpdatePermission.class); - assertEquals(37, classes.size()); + assertEquals(36, classes.size()); for (Class cls : classes) { assertTrue(cls.isAnnotationPresent(ReadPermission.class) || cls.isAnnotationPresent(UpdatePermission.class)); diff --git a/elide-core/src/test/java/com/yahoo/elide/parsers/expression/PermissionExpressionVisitorTest.java b/elide-core/src/test/java/com/yahoo/elide/parsers/expression/PermissionExpressionVisitorTest.java index c83f8b28e3..1e7b05f9d5 100644 --- a/elide-core/src/test/java/com/yahoo/elide/parsers/expression/PermissionExpressionVisitorTest.java +++ b/elide-core/src/test/java/com/yahoo/elide/parsers/expression/PermissionExpressionVisitorTest.java @@ -145,7 +145,7 @@ public ExpressionResult evaluate(EvaluationMode ignored) { if (check instanceof UserCheck) { result = ((UserCheck) check).ok(null); } else { - result = check.ok(null, null, null); + result = ((OperationCheck) check).ok(null, null, null); } if (result) { 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 d2715395df..19ee3290ac 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; @@ -18,10 +19,10 @@ import com.yahoo.elide.core.RequestScope; import com.yahoo.elide.core.TestDictionary; import com.yahoo.elide.core.exceptions.ForbiddenAccessException; -import com.yahoo.elide.security.checks.CommitCheck; import com.yahoo.elide.security.checks.OperationCheck; import com.yahoo.elide.security.checks.UserCheck; +import com.yahoo.elide.security.permissions.ExpressionResult; import org.junit.jupiter.api.Test; import java.util.Optional; @@ -39,10 +40,12 @@ public void testSuccessfulOperationCheck() throws Exception { @UpdatePermission(expression = "sampleOperation") class Model { } - PersistentResource resource = newResource(new Model(), Model.class); + PersistentResource resource = newResource(new Model(), Model.class, false); RequestScope requestScope = resource.getRequestScope(); ChangeSpec cspec = new ChangeSpec(null, null, null, null); - requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource, cspec); + assertEquals(ExpressionResult.PASS, + requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource, cspec)); + requestScope.getPermissionExecutor().executeCommitChecks(); } @@ -53,25 +56,27 @@ public void testFailOperationCheckAll() throws Exception { @UpdatePermission(expression = "sampleOperation AND deny all") class Model { } - PersistentResource resource = newResource(new Model(), Model.class); + PersistentResource resource = newResource(new Model(), Model.class, false); RequestScope requestScope = resource.getRequestScope(); assertThrows( ForbiddenAccessException.class, () -> requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource)); - requestScope.getPermissionExecutor().executeCommitChecks(); } @Test - public void testFailOperationCheckAny() throws Exception { + public void testFailOperationCheckDeferred() throws Exception { @Entity @Include - @UpdatePermission(expression = "sampleCommit OR sampleOperation") + @UpdatePermission(expression = "sampleOperation") class Model { } - PersistentResource resource = newResource(new Model(), Model.class); + PersistentResource resource = newResource(new Model(), Model.class, true); RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource); - // Update permissions are deferred. In the case of "any," commit checks must execute before failure can be detected + + // Because the object is newly created, the check is DEFERRED. + assertEquals(ExpressionResult.DEFERRED, + requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource)); + assertThrows(ForbiddenAccessException.class, () -> requestScope.getPermissionExecutor().executeCommitChecks()); } @@ -79,41 +84,32 @@ class Model { } public void testSuccessfulCommitChecks() throws Exception { @Entity @Include - @UpdatePermission(expression = "sampleCommit") + @UpdatePermission(expression = "sampleOperation") class Model { } - PersistentResource resource = newResource(new Model(), Model.class); + PersistentResource resource = newResource(new Model(), Model.class, true); RequestScope requestScope = resource.getRequestScope(); ChangeSpec cspec = new ChangeSpec(null, null, null, null); - requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource, cspec); - requestScope.getPermissionExecutor().executeCommitChecks(); - } + // Because the object is newly created, the check is DEFERRED. + assertEquals(ExpressionResult.DEFERRED, + requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource, cspec)); - @Test - public void testFailCommitChecks() throws Exception { - @Entity - @Include - @UpdatePermission(expression = "sampleCommit") - class Model { } - - PersistentResource resource = newResource(new Model(), Model.class); - RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource); - assertThrows(ForbiddenAccessException.class, () -> requestScope.getPermissionExecutor().executeCommitChecks()); + requestScope.getPermissionExecutor().executeCommitChecks(); } @Test public void testReadFieldAwareSuccessAllAnyField() { - PersistentResource resource = newResource(SampleBean.class); + PersistentResource resource = newResource(SampleBean.class, false); RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource, new ChangeSpec(null, null, null, null)); + assertEquals(ExpressionResult.PASS, + requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource, new ChangeSpec(null, null, null, null))); requestScope.getPermissionExecutor().executeCommitChecks(); } @Test public void testReadFieldAwareSuccessFailureAnyField() { - PersistentResource resource = newResource(SampleBean.class); + PersistentResource resource = newResource(SampleBean.class, false); RequestScope requestScope = resource.getRequestScope(); assertThrows( ForbiddenAccessException.class, @@ -123,15 +119,16 @@ public void testReadFieldAwareSuccessFailureAnyField() { @Test public void testReadFieldAwareSuccessAll() { - PersistentResource resource = newResource(SampleBean.class); + PersistentResource resource = newResource(SampleBean.class, false); RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, new ChangeSpec(null, null, null, null), ReadPermission.class, "allVisible"); + assertEquals(ExpressionResult.PASS, + requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, new ChangeSpec(null, null, null, null), ReadPermission.class, "allVisible")); requestScope.getPermissionExecutor().executeCommitChecks(); } @Test public void testReadFieldAwareFailureAllSpecificField() { - PersistentResource resource = newResource(SampleBean.class); + PersistentResource resource = newResource(SampleBean.class, false); RequestScope requestScope = resource.getRequestScope(); assertThrows( ForbiddenAccessException.class, @@ -142,7 +139,7 @@ public void testReadFieldAwareFailureAllSpecificField() { @Test public void testReadFieldAwareFailureAllNoOverride() { - PersistentResource resource = newResource(SampleBean.class); + PersistentResource resource = newResource(SampleBean.class, false); RequestScope requestScope = resource.getRequestScope(); assertThrows( ForbiddenAccessException.class, @@ -153,7 +150,7 @@ public void testReadFieldAwareFailureAllNoOverride() { @Test public void testReadFieldAwareFailureAll() { - PersistentResource resource = newResource(SampleBean.class); + PersistentResource resource = newResource(SampleBean.class, false); RequestScope requestScope = resource.getRequestScope(); assertThrows( ForbiddenAccessException.class, @@ -164,15 +161,16 @@ public void testReadFieldAwareFailureAll() { @Test public void testReadFieldAwareSuccessAny() { - PersistentResource resource = newResource(SampleBean.class); + PersistentResource resource = newResource(SampleBean.class, false); RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, new ChangeSpec(null, null, null, null), ReadPermission.class, "mayFailInCommit"); + assertEquals(ExpressionResult.PASS, + requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, new ChangeSpec(null, null, null, null), ReadPermission.class, "mayFailInCommit")); requestScope.getPermissionExecutor().executeCommitChecks(); } @Test public void testReadFieldAwareFailureAny() { - PersistentResource resource = newResource(SampleBean.class); + PersistentResource resource = newResource(SampleBean.class, false); RequestScope requestScope = resource.getRequestScope(); assertThrows( ForbiddenAccessException.class, @@ -183,15 +181,16 @@ public void testReadFieldAwareFailureAny() { @Test public void testUpdateFieldAwareSuccessAll() { - PersistentResource resource = newResource(SampleBean.class); + PersistentResource resource = newResource(SampleBean.class, true); RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, new ChangeSpec(null, null, null, null), UpdatePermission.class, "allVisible"); + assertEquals(ExpressionResult.DEFERRED, + requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, new ChangeSpec(null, null, null, null), UpdatePermission.class, "allVisible")); requestScope.getPermissionExecutor().executeCommitChecks(); } @Test public void testUpdateFieldAwareFailureAll() { - PersistentResource resource = newResource(SampleBean.class); + PersistentResource resource = newResource(SampleBean.class, true); RequestScope requestScope = resource.getRequestScope(); requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, null, UpdatePermission.class, "allVisible"); assertThrows(ForbiddenAccessException.class, () -> requestScope.getPermissionExecutor().executeCommitChecks()); @@ -199,15 +198,16 @@ public void testUpdateFieldAwareFailureAll() { @Test public void testUpdateFieldAwareSuccessAny() { - PersistentResource resource = newResource(SampleBean.class); + PersistentResource resource = newResource(SampleBean.class, true); RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, new ChangeSpec(null, null, null, null), UpdatePermission.class, "mayFailInCommit"); + assertEquals(ExpressionResult.DEFERRED, + requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, new ChangeSpec(null, null, null, null), UpdatePermission.class, "mayFailInCommit")); requestScope.getPermissionExecutor().executeCommitChecks(); } @Test public void testUpdateFieldAwareFailureAny() { - PersistentResource resource = newResource(SampleBean.class); + PersistentResource resource = newResource(SampleBean.class, true); RequestScope requestScope = resource.getRequestScope(); requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, null, UpdatePermission.class, "mayFailInCommit"); assertThrows(ForbiddenAccessException.class, () -> requestScope.getPermissionExecutor().executeCommitChecks()); @@ -215,16 +215,16 @@ public void testUpdateFieldAwareFailureAny() { @Test public void testReadFieldAwareEntireOpenBean() { - PersistentResource resource = newResource(OpenBean.class); + PersistentResource resource = newResource(OpenBean.class, false); RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource); - requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, null, ReadPermission.class, "open"); + assertEquals(ExpressionResult.PASS, requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource)); + assertEquals(ExpressionResult.PASS, requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, null, ReadPermission.class, "open")); requestScope.getPermissionExecutor().executeCommitChecks(); } @Test public void testReadFailureFieldAwareOpenBean() { - PersistentResource resource = newResource(OpenBean.class); + PersistentResource resource = newResource(OpenBean.class, false); RequestScope requestScope = resource.getRequestScope(); assertThrows( ForbiddenAccessException.class, @@ -237,18 +237,19 @@ public void testReadFailureFieldAwareOpenBean() { public void testPassAnyFieldAwareFailOperationSuccessCommit() { @Entity @Include - @UpdatePermission(expression = "deny all AND passingCommit") + @UpdatePermission(expression = "deny all AND passingOp") class Model { @Id public Long id; - @UpdatePermission(expression = "deny all OR passingCommit") + @UpdatePermission(expression = "deny all OR passingOp") public String field = "some data"; } - PersistentResource resource = newResource(new Model(), Model.class); + PersistentResource resource = newResource(new Model(), Model.class, true); RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource); + assertEquals(ExpressionResult.DEFERRED, + requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource)); requestScope.getPermissionExecutor().executeCommitChecks(); } @@ -261,13 +262,14 @@ class Model { @Id public Long id; - @UpdatePermission(expression = "allow all AND FailAtCommit") + @UpdatePermission(expression = "allow all AND FailOp") public String field = "some data"; } - PersistentResource resource = newResource(new Model(), Model.class); + PersistentResource resource = newResource(new Model(), Model.class, true); RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource); + assertEquals(ExpressionResult.DEFERRED, + requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource)); assertThrows(ForbiddenAccessException.class, () -> requestScope.getPermissionExecutor().executeCommitChecks()); } @@ -275,18 +277,19 @@ class Model { public void testPassAnySpecificFieldAwareFailOperationSuccessCommit() { @Entity @Include - @UpdatePermission(expression = "deny all AND passingCommit") + @UpdatePermission(expression = "deny all AND passingOp") class Model { @Id public Long id; - @UpdatePermission(expression = "deny all OR passingCommit") + @UpdatePermission(expression = "deny all OR passingOp") public String field = "some data"; } - PersistentResource resource = newResource(new Model(), Model.class); + PersistentResource resource = newResource(new Model(), Model.class, true); RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, null, UpdatePermission.class, "field"); + assertEquals(ExpressionResult.DEFERRED, + requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, null, UpdatePermission.class, "field")); requestScope.getPermissionExecutor().executeCommitChecks(); } @@ -299,13 +302,14 @@ class Model { @Id public Long id; - @UpdatePermission(expression = "allow all AND FailAtCommit") + @UpdatePermission(expression = "allow all AND FailOp") public String field = "some data"; } - PersistentResource resource = newResource(new Model(), Model.class); + PersistentResource resource = newResource(new Model(), Model.class, true); RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, null, UpdatePermission.class, "field"); + assertEquals(ExpressionResult.DEFERRED, + requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, null, UpdatePermission.class, "field")); assertThrows(ForbiddenAccessException.class, () -> requestScope.getPermissionExecutor().executeCommitChecks()); } @@ -316,7 +320,7 @@ public void testBadInstance() { @UpdatePermission(expression = "privatePermission") class Model { } - PersistentResource resource = newResource(new Model(), Model.class); + PersistentResource resource = newResource(new Model(), Model.class, true); RequestScope requestScope = resource.getRequestScope(); assertThrows( IllegalArgumentException.class, @@ -326,70 +330,65 @@ class Model { } @Test public void testSpecificFieldOveriddenOperationCheckSucceed() { - PersistentResource resource = newResource(CommitCheckEntity.class); + PersistentResource resource = newResource(CheckedEntity.class, true); RequestScope requestScope = resource.getRequestScope(); // Should succeed in operation check despite the commit check failure - requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, null, UpdatePermission.class, "hello"); + assertEquals(ExpressionResult.DEFERRED, + requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, null, UpdatePermission.class, "hello")); requestScope.getPermissionExecutor().executeCommitChecks(); } @Test public void testSpecificFieldCommitCheckFailByOveriddenField() { - PersistentResource resource = newResource(CommitCheckEntity.class); + PersistentResource resource = newResource(CheckedEntity.class, true); RequestScope requestScope = resource.getRequestScope(); - // Should succeed in commit check despite the operation check failure + assertEquals(ExpressionResult.DEFERRED, + requestScope.getPermissionExecutor().checkSpecificFieldPermissions(resource, new ChangeSpec(null, null, null, null), UpdatePermission.class, "hello")); assertThrows( ForbiddenAccessException.class, - () -> requestScope.getPermissionExecutor().checkSpecificFieldPermissions( - resource, new ChangeSpec(null, null, null, null), UpdatePermission.class, "hello")); - requestScope.getPermissionExecutor().executeCommitChecks(); + () -> requestScope.getPermissionExecutor().executeCommitChecks()); } @Test - public void testReadCheckExpressionAlwaysInline() { + public void testReadCheckExpressionForNewlyCreatedObject() { @Entity @Include - @ReadPermission(expression = "FailAtCommit") + @ReadPermission(expression = "FailOp") class Model { } - PersistentResource resource = newResource(new Model(), Model.class); + PersistentResource resource = newResource(new Model(), Model.class, true); RequestScope requestScope = resource.getRequestScope(); requestScope.getDictionary().bindEntity(Model.class); - assertThrows( - ForbiddenAccessException.class, - () -> requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource)); - // NOTE: This check should throw a ForbiddenAccess since commit-time checks should be converted - // to inline checks. As a result, DO NOT call executeCommitChecks() in this test. + assertEquals(ExpressionResult.DEFERRED, requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource)); + assertThrows(ForbiddenAccessException.class, () -> requestScope.getPermissionExecutor().executeCommitChecks()); } @Test - public void testDeleteCheckExpressionAlwaysInline() { + public void testDeleteCheckExpressionForNewlyCreatedObject() { @Entity @Include - @DeletePermission(expression = "FailAtCommit") + @DeletePermission(expression = "FailOp") class Model { } - PersistentResource resource = newResource(new Model(), Model.class); + PersistentResource resource = newResource(new Model(), Model.class, true); RequestScope requestScope = resource.getRequestScope(); requestScope.getDictionary().bindEntity(Model.class); - assertThrows( - ForbiddenAccessException.class, - () -> requestScope.getPermissionExecutor().checkPermission(DeletePermission.class, resource)); - // NOTE: This check should throw a ForbiddenAccess since commit-time checks should be converted - // to inline checks. As a result, DO NOT call executeCommitChecks() in this test. + assertEquals(ExpressionResult.DEFERRED, requestScope.getPermissionExecutor().checkPermission(DeletePermission.class, resource)); + assertThrows(ForbiddenAccessException.class, () -> requestScope.getPermissionExecutor().executeCommitChecks()); } @Test public void testCache() { - PersistentResource resource = newResource(AnnotationOnlyRecord.class); + PersistentResource resource = newResource(AnnotationOnlyRecord.class, false); RequestScope requestScope = resource.getRequestScope(); - requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource); - requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource); + assertEquals(ExpressionResult.PASS, requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource)); + assertEquals(ExpressionResult.PASS, requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource)); } + @Test public void testNoCache() { - PersistentResource resource = newResource(AnnotationOnlyRecord.class); + PersistentResource resource = newResource(AnnotationOnlyRecord.class, false); RequestScope requestScope = resource.getRequestScope(); ChangeSpec cspec = new ChangeSpec(null, null, null, null); assertThrows( @@ -402,30 +401,31 @@ public void testNoCache() { @Test public void testUserCheckCache() { - PersistentResource resource = newResource(UserCheckCacheRecord.class); + PersistentResource resource = newResource(UserCheckCacheRecord.class, false); RequestScope requestScope = resource.getRequestScope(); ChangeSpec cspec = new ChangeSpec(null, null, null, null); // This should cache for updates, reads, etc. - requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource, cspec); - requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource, cspec); - requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource, cspec); - requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource, cspec); + assertEquals(ExpressionResult.PASS, requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource, cspec)); + assertEquals(ExpressionResult.PASS, requestScope.getPermissionExecutor().checkPermission(UpdatePermission.class, resource, cspec)); + assertEquals(ExpressionResult.PASS, requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource, cspec)); + assertEquals(ExpressionResult.PASS, requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource, cspec)); } - public PersistentResource newResource(T obj, Class cls) { + public PersistentResource newResource(T obj, Class cls, boolean markNew) { EntityDictionary dictionary = TestDictionary.getTestDictionary(); dictionary.bindEntity(cls); RequestScope requestScope = new RequestScope(null, null, null, null, null, getElideSettings(dictionary)); - return new PersistentResource<>(obj, null, requestScope.getUUIDFor(obj), requestScope); + PersistentResource resource = new PersistentResource<>(obj, null, requestScope.getUUIDFor(obj), requestScope); + if (markNew) { + requestScope.getNewPersistentResources().add(resource); + } + return resource; } - public PersistentResource newResource(Class cls) { - EntityDictionary dictionary = TestDictionary.getTestDictionary(); - dictionary.bindEntity(cls); - RequestScope requestScope = new RequestScope(null, null, null, null, null, getElideSettings(dictionary)); + public PersistentResource newResource(Class cls, boolean markNew) { try { Object obj = cls.newInstance(); - return new PersistentResource<>(obj, null, requestScope.getUUIDFor(obj), requestScope); + return newResource(obj, cls, markNew); } catch (InstantiationException | IllegalAccessException e) { return null; } @@ -444,39 +444,14 @@ public boolean ok(Object object, com.yahoo.elide.security.RequestScope requestSc } } - public static final class SampleCommitCheck extends CommitCheck { - @Override - public boolean ok(Object object, com.yahoo.elide.security.RequestScope requestScope, Optional changeSpec) { - return changeSpec.isPresent(); - } - } - public static final class SampleOperationCheckCommitInverse extends OperationCheck { + public static final class SampleOperationCheckInverse extends OperationCheck { @Override public boolean ok(Object object, com.yahoo.elide.security.RequestScope requestScope, Optional changeSpec) { return !changeSpec.isPresent(); } } - public static final class PassingCommitCheck extends CommitCheck { - @Override - public boolean ok(Object object, com.yahoo.elide.security.RequestScope requestScope, Optional changeSpec) { - return true; - } - } - - public static final class FailingCommitCheck extends CommitCheck { - @Override - public boolean ok(Object object, com.yahoo.elide.security.RequestScope requestScope, Optional changeSpec) { - return false; - } - - @Override - public String checkIdentifier() { - return "FailAtCommit"; - } - } - @ReadPermission(expression = "deny all") @UpdatePermission(expression = "deny all") @Include @@ -486,7 +461,7 @@ public static final class SampleBean { public Long id; @ReadPermission(expression = "allow all AND sampleOperation") - @UpdatePermission(expression = "allow all AND sampleCommit") + @UpdatePermission(expression = "allow all AND sampleOperation") public String allVisible = "You should see me!"; public String defaultHidden = "I'm invisible. muwahaha..."; @@ -496,7 +471,7 @@ public static final class SampleBean { public String cannotSeeMe = "hidden"; @ReadPermission(expression = "sampleOperation") - @UpdatePermission(expression = "sampleCommit OR deny all") + @UpdatePermission(expression = "sampleOperation OR deny all") public String mayFailInCommit = "aw :("; } @@ -510,23 +485,23 @@ public static final class OpenBean { public String open; - @ReadPermission(expression = "deny all AND sampleOperation") - @UpdatePermission(expression = "sampleCommit AND sampleOperation") + @ReadPermission(expression = "allow all AND sampleOperation") + @UpdatePermission(expression = "allow all AND sampleOperation") public String openAll = "all"; - @ReadPermission(expression = "sampleOperation OR sampleOperation") - @UpdatePermission(expression = "sampleCommit OR sampleOperation") + @ReadPermission(expression = "deny all OR sampleOperation") + @UpdatePermission(expression = "deny all OR sampleOperation") public String openAny = "all"; } @Entity @Include - @UpdatePermission(expression = "sampleCommit") - public static final class CommitCheckEntity { + @UpdatePermission(expression = "sampleOperation") + public static final class CheckedEntity { @Id public Long id; - @UpdatePermission(expression = "sampleOperationCommitInverse") + @UpdatePermission(expression = "sampleOperationInverse") public String hello; } @@ -540,6 +515,20 @@ public boolean ok(Object object, com.yahoo.elide.security.RequestScope requestSc } } + public static final class PassingOperationCheck extends OperationCheck { + @Override + public boolean ok(Object object, com.yahoo.elide.security.RequestScope requestScope, Optional changeSpec) { + return true; + } + } + + public static final class FailingOperationCheck extends OperationCheck { + @Override + public boolean ok(Object object, com.yahoo.elide.security.RequestScope requestScope, Optional changeSpec) { + return false; + } + } + @Entity @Include @ReadPermission(expression = "shouldCache") diff --git a/elide-core/src/test/java/example/Child.java b/elide-core/src/test/java/example/Child.java index e7664b78df..407a85a2d1 100644 --- a/elide-core/src/test/java/example/Child.java +++ b/elide-core/src/test/java/example/Child.java @@ -12,7 +12,6 @@ import com.yahoo.elide.annotation.SharePermission; import com.yahoo.elide.security.ChangeSpec; import com.yahoo.elide.security.RequestScope; -import com.yahoo.elide.security.checks.CommitCheck; import com.yahoo.elide.security.checks.OperationCheck; import com.fasterxml.jackson.annotation.JsonIgnore; @@ -32,7 +31,7 @@ @Entity(name = "childEntity") @CreatePermission(expression = "initCheck") @SharePermission -@ReadPermission(expression = "negativeChildId AND negativeIntegerUser AND initCheckOp") +@ReadPermission(expression = "negativeChildId AND negativeIntegerUser AND initCheck") @Include(rootLevel = true, type = "child") @Audit(action = Audit.Action.DELETE, operation = 0, @@ -114,7 +113,7 @@ public void setReadNoAccess(Child noReadAccess) { this.noReadAccess = noReadAccess; } - static public class InitCheck extends CommitCheck { + static public class InitCheck extends OperationCheck { @Override public boolean ok(Child child, RequestScope requestScope, Optional changeSpec) { if (child.getParents() != null) { @@ -122,25 +121,5 @@ public boolean ok(Child child, RequestScope requestScope, Optional c } return false; } - - @Override - public String checkIdentifier() { - return "initCheck"; - } - } - - static public class InitCheckOp extends OperationCheck { - @Override - public boolean ok(Child child, RequestScope requestScope, Optional changeSpec) { - if (child.getParents() != null) { - return true; - } - return false; - } - - @Override - public String checkIdentifier() { - return "initCheckOp"; - } } } diff --git a/elide-core/src/test/java/example/NegativeChildIdCheck.java b/elide-core/src/test/java/example/NegativeChildIdCheck.java index 39691db950..f516fa32ee 100644 --- a/elide-core/src/test/java/example/NegativeChildIdCheck.java +++ b/elide-core/src/test/java/example/NegativeChildIdCheck.java @@ -19,9 +19,4 @@ public class NegativeChildIdCheck extends OperationCheck { public boolean ok(Child child, RequestScope requestScope, Optional fChangeSpec) { return child.getId() >= 0; } - - @Override - public String checkIdentifier() { - return "negativeChildId"; - } } diff --git a/elide-core/src/test/java/example/NegativeIntegerUserCheck.java b/elide-core/src/test/java/example/NegativeIntegerUserCheck.java index 155e0daad4..3508b5c772 100644 --- a/elide-core/src/test/java/example/NegativeIntegerUserCheck.java +++ b/elide-core/src/test/java/example/NegativeIntegerUserCheck.java @@ -17,9 +17,4 @@ public boolean ok(User user) { Integer id = (Integer) user.getOpaqueUser(); return id >= 0; } - - @Override - public String checkIdentifier() { - return "negativeIntegerUser"; - } } diff --git a/elide-core/src/test/java/example/Parent.java b/elide-core/src/test/java/example/Parent.java index 51b379fe07..d2bf6ca5f1 100644 --- a/elide-core/src/test/java/example/Parent.java +++ b/elide-core/src/test/java/example/Parent.java @@ -12,7 +12,6 @@ import com.yahoo.elide.annotation.UpdatePermission; import com.yahoo.elide.security.ChangeSpec; import com.yahoo.elide.security.RequestScope; -import com.yahoo.elide.security.checks.CommitCheck; import com.yahoo.elide.security.checks.OperationCheck; import lombok.ToString; @@ -30,9 +29,9 @@ import javax.validation.constraints.NotNull; @CreatePermission(expression = "parentInitCheck OR allow all") -@ReadPermission(expression = "parentInitCheckOp OR allow all") +@ReadPermission(expression = "parentInitCheck OR allow all") @UpdatePermission(expression = "parentInitCheck OR allow all OR deny all") -@DeletePermission(expression = "parentInitCheckOp OR allow all OR deny all") +@DeletePermission(expression = "parentInitCheck OR allow all OR deny all") @Include(rootLevel = true, type = "parent") // optional here because class has this name @Entity @ToString @@ -100,22 +99,7 @@ public void setSpecialAttribute(String specialAttribute) { this.specialAttribute = specialAttribute; } - static public class InitCheck extends CommitCheck { - @Override - public String checkIdentifier() { - return "parentInitCheck"; - } - - @Override - public boolean ok(Parent parent, RequestScope requestScope, Optional changeSpec) { - if (parent.getChildren() != null && parent.getSpouses() != null) { - return true; - } - return false; - } - } - - static public class InitCheckOp extends OperationCheck { + static public class InitCheck extends OperationCheck { @Override public boolean ok(Parent parent, RequestScope requestScope, Optional changeSpec) { if (parent.getChildren() != null && parent.getSpouses() != null) { @@ -123,11 +107,6 @@ public boolean ok(Parent parent, RequestScope requestScope, Optional } return false; } - - @Override - public String checkIdentifier() { - return "parentInitCheckOp"; - } } static public class SpecialValue extends OperationCheck { @@ -142,10 +121,5 @@ public boolean ok(Parent object, RequestScope requestScope, Optional } return false; } - - @Override - public String checkIdentifier() { - return "specialValue"; - } } } diff --git a/elide-core/src/test/java/example/TestCheckMappings.java b/elide-core/src/test/java/example/TestCheckMappings.java index 5c64cee419..32e055cbe9 100644 --- a/elide-core/src/test/java/example/TestCheckMappings.java +++ b/elide-core/src/test/java/example/TestCheckMappings.java @@ -27,19 +27,16 @@ public class TestCheckMappings { .put("changeSpecCollection", PersistentResourceTest.ChangeSpecCollection.class) .put("changeSpecNonCollection", PersistentResourceTest.ChangeSpecNonCollection.class) .put("initCheck", Child.InitCheck.class) - .put("initCheckOp", Child.InitCheckOp.class) .put("parentInitCheck", Parent.InitCheck.class) - .put("parentInitCheckOp", Parent.InitCheckOp.class) .put("negativeIntegerUser", NegativeIntegerUserCheck.class) .put("negativeChildId", NegativeChildIdCheck.class) - .put("FailAtCommit", PermissionExecutorTest.FailingCommitCheck.class) + .put("FailOp", PermissionExecutorTest.FailingOperationCheck.class) .put("privatePermission", PrivatePermission.class) .put("sampleOperation", PermissionExecutorTest.SampleOperationCheck.class) - .put("sampleCommit", PermissionExecutorTest.SampleCommitCheck.class) - .put("sampleOperationCommitInverse", PermissionExecutorTest.SampleOperationCheckCommitInverse.class) + .put("sampleOperationInverse", PermissionExecutorTest.SampleOperationCheckInverse.class) .put("shouldCache", PermissionExecutorTest.ShouldCache.class) .put("peUserCheck", PermissionExecutorTest.UserCheckTest.class) - .put("passingCommit", PermissionExecutorTest.PassingCommitCheck.class) + .put("passingOp", PermissionExecutorTest.PassingOperationCheck.class) .put("Principal is user one", UserIdChecks.UserOneCheck.class) .put("Principal is user two", UserIdChecks.UserTwoCheck.class) .put("Principal is user three", UserIdChecks.UserThreeCheck.class) diff --git a/elide-core/src/test/java/example/UserIdChecks.java b/elide-core/src/test/java/example/UserIdChecks.java index a49bb89d6f..e3700f877b 100644 --- a/elide-core/src/test/java/example/UserIdChecks.java +++ b/elide-core/src/test/java/example/UserIdChecks.java @@ -19,11 +19,6 @@ public boolean ok(User user) { Integer id = (Integer) user.getOpaqueUser(); return id.equals(1); } - - @Override - public String checkIdentifier() { - return "UserOne"; - } } public static class UserTwoCheck extends UserCheck { @@ -32,11 +27,6 @@ public boolean ok(User user) { Integer id = (Integer) user.getOpaqueUser(); return id.equals(2); } - - @Override - public String checkIdentifier() { - return "UserTwo"; - } } public static class UserThreeCheck extends UserCheck { @@ -45,11 +35,6 @@ public boolean ok(User user) { Integer id = (Integer) user.getOpaqueUser(); return id.equals(3); } - - @Override - public String checkIdentifier() { - return "UserThree"; - } } public static class UserFourCheck extends UserCheck { @@ -58,10 +43,5 @@ public boolean ok(User user) { Integer id = (Integer) user.getOpaqueUser(); return id.equals(4); } - - @Override - public String checkIdentifier() { - return "UserFour"; - } } } diff --git a/elide-graphql/src/test/java/graphqlEndpointTestModels/security/CommitChecks.java b/elide-graphql/src/test/java/graphqlEndpointTestModels/security/CommitChecks.java index aa4054535c..fb5024b122 100644 --- a/elide-graphql/src/test/java/graphqlEndpointTestModels/security/CommitChecks.java +++ b/elide-graphql/src/test/java/graphqlEndpointTestModels/security/CommitChecks.java @@ -6,7 +6,7 @@ package graphqlEndpointTestModels.security; import com.yahoo.elide.security.RequestScope; -import com.yahoo.elide.security.checks.CommitCheck; +import com.yahoo.elide.security.checks.OperationCheck; import java.security.Principal; import java.util.Optional; @@ -14,7 +14,7 @@ public abstract class CommitChecks { public static final String IS_NOT_USER_3 = "isnt user three"; - public static class IsNotUser3 extends CommitCheck { + public static class IsNotUser3 extends OperationCheck { @Override public boolean ok(Object object, RequestScope requestScope, Optional optional) { diff --git a/elide-integration-tests/src/test/java/example/Child.java b/elide-integration-tests/src/test/java/example/Child.java index 59895c18d2..2ce4c9587c 100644 --- a/elide-integration-tests/src/test/java/example/Child.java +++ b/elide-integration-tests/src/test/java/example/Child.java @@ -18,7 +18,6 @@ import com.yahoo.elide.security.ChangeSpec; import com.yahoo.elide.security.FilterExpressionCheck; import com.yahoo.elide.security.RequestScope; -import com.yahoo.elide.security.checks.CommitCheck; import com.yahoo.elide.security.checks.OperationCheck; import java.util.Optional; @@ -37,7 +36,7 @@ @Entity(name = "childEntity") @CreatePermission(expression = "initCheck") @SharePermission -@ReadPermission(expression = "negativeChildId AND negativeIntegerUser AND initCheckOp AND initCheckFilter") +@ReadPermission(expression = "negativeChildId AND negativeIntegerUser AND initCheck AND initCheckFilter") @Include(rootLevel = true, type = "child") @Audit(action = Audit.Action.DELETE, operation = 0, @@ -115,20 +114,7 @@ public String setComputedFailTest(String unused) { throw new IllegalAccessError(); } - /** - * Initialization validation check. - */ - static public class InitCheck extends CommitCheck { - @Override - public boolean ok(Child child, RequestScope requestScope, Optional changeSpec) { - if (child.getParents() != null) { - return true; - } - return false; - } - } - - static public class InitCheckOp extends OperationCheck { + static public class InitCheck extends OperationCheck { @Override public boolean ok(Child child, RequestScope requestScope, Optional changeSpec) { if (child.getParents() != null) { diff --git a/elide-integration-tests/src/test/java/example/NoCommitEntity.java b/elide-integration-tests/src/test/java/example/NoCommitEntity.java index 327e188b14..48c1d23612 100644 --- a/elide-integration-tests/src/test/java/example/NoCommitEntity.java +++ b/elide-integration-tests/src/test/java/example/NoCommitEntity.java @@ -10,7 +10,7 @@ import com.yahoo.elide.annotation.UpdatePermission; import com.yahoo.elide.security.ChangeSpec; import com.yahoo.elide.security.RequestScope; -import com.yahoo.elide.security.checks.CommitCheck; +import com.yahoo.elide.security.checks.OperationCheck; import java.util.Optional; @@ -27,7 +27,7 @@ @Entity @Table(name = "nocommit") public class NoCommitEntity extends BaseId { - static public class NoCommitCheck extends CommitCheck { + static public class NoCommitCheck extends OperationCheck { @Override public boolean ok(T record, RequestScope requestScope, Optional changeSpec) { return false; diff --git a/elide-integration-tests/src/test/java/example/Parent.java b/elide-integration-tests/src/test/java/example/Parent.java index f49b1ac1d9..18cfb108ea 100644 --- a/elide-integration-tests/src/test/java/example/Parent.java +++ b/elide-integration-tests/src/test/java/example/Parent.java @@ -14,7 +14,6 @@ import com.yahoo.elide.annotation.UpdatePermission; import com.yahoo.elide.security.ChangeSpec; import com.yahoo.elide.security.RequestScope; -import com.yahoo.elide.security.checks.CommitCheck; import com.yahoo.elide.security.checks.OperationCheck; import lombok.ToString; @@ -33,9 +32,9 @@ * Parent test bean. */ @CreatePermission(expression = "parentInitCheck OR allow all") -@ReadPermission(expression = "parentInitCheckOp OR allow all") +@ReadPermission(expression = "parentInitCheck OR allow all") @UpdatePermission(expression = "parentInitCheck OR allow all OR deny all") -@DeletePermission(expression = "parentInitCheckOp OR allow all OR deny all") +@DeletePermission(expression = "parentInitCheck OR allow all OR deny all") @SharePermission @Include(rootLevel = true, type = "parent") // optional here because class has this name @Paginate(maxLimit = 100000) @@ -104,20 +103,8 @@ public void setSpecialAttribute(String specialAttribute) { this.specialAttribute = specialAttribute; } - /** - * Initialization validation check. - */ - static public class InitCheck extends CommitCheck { - @Override - public boolean ok(Parent parent, RequestScope requestScope, Optional changeSpec) { - if (parent.getChildren() != null && parent.getSpouses() != null) { - return true; - } - return false; - } - } - static public class InitCheckOp extends OperationCheck { + static public class InitCheck extends OperationCheck { @Override public boolean ok(Parent parent, RequestScope requestScope, Optional changeSpec) { if (parent.getChildren() != null && parent.getSpouses() != null) { diff --git a/elide-integration-tests/src/test/java/example/TestCheckMappings.java b/elide-integration-tests/src/test/java/example/TestCheckMappings.java index 83a4d7094b..277c1979de 100644 --- a/elide-integration-tests/src/test/java/example/TestCheckMappings.java +++ b/elide-integration-tests/src/test/java/example/TestCheckMappings.java @@ -23,11 +23,9 @@ public class TestCheckMappings { .put("deny all", Role.NONE.class) .put("adminRoleCheck", User.AdminRoleCheck.class) .put("initCheck", Child.InitCheck.class) - .put("initCheckOp", Child.InitCheckOp.class) .put("FailCheckOp", Child.FailCheckOp.class) .put("initCheckFilter", Child.InitCheckFilter.class) .put("parentInitCheck", Parent.InitCheck.class) - .put("parentInitCheckOp", Parent.InitCheckOp.class) .put("parentSpecialValue", Parent.SpecialValue.class) .put("negativeIntegerUser", NegativeIntegerUserCheck.class) .put("negativeChildId", NegativeChildIdCheck.class) diff --git a/elide-integration-tests/src/test/java/example/User.java b/elide-integration-tests/src/test/java/example/User.java index a48e92d986..dedb7cf767 100644 --- a/elide-integration-tests/src/test/java/example/User.java +++ b/elide-integration-tests/src/test/java/example/User.java @@ -77,10 +77,5 @@ static public class AdminRoleCheck extends OperationCheck { public boolean ok(User user, RequestScope requestScope, Optional changeSpec) { return (user.getRole() == 1); } - - @Override - public String checkIdentifier() { - return "adminRoleCheck"; - } } }