From 7b7aba5cc1070f1e337348083f923708e75f2be5 Mon Sep 17 00:00:00 2001 From: Aaron Klish Date: Thu, 19 Sep 2019 15:29:20 -0500 Subject: [PATCH] Fix GraphQL Nested UPDATE bug. (#974) * Fix bug in GraphQL where we add the parent relationship during an UPDATE request * Reverting owasp check back to level 6 * Inspection rework --- .../java/com/yahoo/elide/security/PersistentResource.java | 8 ++++++++ .../java/com/yahoo/elide/core/PersistentResource.java | 7 ++----- .../security/executors/ActivePermissionExecutor.java | 2 +- .../yahoo/elide/graphql/PersistentResourceFetcher.java | 4 +++- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/elide-annotations/src/main/java/com/yahoo/elide/security/PersistentResource.java b/elide-annotations/src/main/java/com/yahoo/elide/security/PersistentResource.java index b05663f07f..557e24c673 100644 --- a/elide-annotations/src/main/java/com/yahoo/elide/security/PersistentResource.java +++ b/elide-annotations/src/main/java/com/yahoo/elide/security/PersistentResource.java @@ -22,4 +22,12 @@ public interface PersistentResource { T getObject(); Class getResourceClass(); RequestScope getRequestScope(); + + /** + * Returns whether or not this resource was created in this transaction. + * @return True if this resource is newly created. + */ + default boolean isNewlyCreated() { + return getRequestScope().getNewResources().contains(this); + } } diff --git a/elide-core/src/main/java/com/yahoo/elide/core/PersistentResource.java b/elide-core/src/main/java/com/yahoo/elide/core/PersistentResource.java index 69aefebe5c..da37a5eb06 100644 --- a/elide-core/src/main/java/com/yahoo/elide/core/PersistentResource.java +++ b/elide-core/src/main/java/com/yahoo/elide/core/PersistentResource.java @@ -95,7 +95,7 @@ public String toString() { return String.format("PersistentResource{type=%s, id=%s}", type, uuid.orElse(getId())); } - /** + /** * Create a resource in the database. * @param parent - The immediate ancestor in the lineage or null if this is a root. * @param entityClass the entity class @@ -1270,8 +1270,6 @@ protected Map getAttributes() { */ protected void setValueChecked(String fieldName, Object newValue) { Object existingValue = getValueUnchecked(fieldName); - ChangeSpec spec = new ChangeSpec(this, fieldName, existingValue, newValue); - boolean isNewlyCreated = requestScope.getNewPersistentResources().contains(this); // TODO: Need to refactor this logic. For creates this is properly converted in the executor. This logic // should be explicitly encapsulated here, not there. @@ -1587,8 +1585,7 @@ protected Set filterFields(Collection fields) { */ private void triggerUpdate(String fieldName, Object original, Object value) { ChangeSpec changeSpec = new ChangeSpec(this, fieldName, original, value); - boolean isNewlyCreated = requestScope.getNewPersistentResources().contains(this); - CRUDEvent.CRUDAction action = isNewlyCreated + CRUDEvent.CRUDAction action = isNewlyCreated() ? CRUDEvent.CRUDAction.CREATE : CRUDEvent.CRUDAction.UPDATE; 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 502819828e..7266d5bcc9 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 @@ -126,7 +126,7 @@ public ExpressionResult checkPermission(Class annotati Function expressionExecutor = (expression) -> { // for newly created object in PatchRequest limit to User checks - if (requestScope.getNewPersistentResources().contains(resource)) { + if (resource.isNewlyCreated()) { return executeUserChecksDeferInline(annotationClass, expression); } return executeExpressions(expression, annotationClass, Expression.EvaluationMode.INLINE_CHECKS_ONLY); diff --git a/elide-graphql/src/main/java/com/yahoo/elide/graphql/PersistentResourceFetcher.java b/elide-graphql/src/main/java/com/yahoo/elide/graphql/PersistentResourceFetcher.java index fbec5c1aa6..1ea7bb5096 100644 --- a/elide-graphql/src/main/java/com/yahoo/elide/graphql/PersistentResourceFetcher.java +++ b/elide-graphql/src/main/java/com/yahoo/elide/graphql/PersistentResourceFetcher.java @@ -298,7 +298,9 @@ private ConnectionContainer upsertOrUpdateObjects(Environment context, /* fixup relationships */ for (Entity entity : entitySet) { graphWalker(entity, this::updateRelationship); - if (!context.isRoot()) { /* add relation between parent and nested entity */ + PersistentResource childResource = entity.toPersistentResource(); + if (!context.isRoot() && childResource.isNewlyCreated()) { + /* add relation between parent and nested entity */ context.parentResource.addRelation(context.field.getName(), entity.toPersistentResource()); } }