From 3baf9bc722da1d4d9790a870d6453a7945c27fea Mon Sep 17 00:00:00 2001 From: t-burch <119930761+t-burch@users.noreply.github.com> Date: Wed, 25 Oct 2023 17:21:50 +0200 Subject: [PATCH] Added limit option for amount of mutations in a GraphQL query document (#753) * Fixed wording * Fixed wording * Removed unnecessary check that lead to a false detection of an error * Changed HTTP status code in maxMutations error * Refactored countMutations into lamba method and added test for it * Added missing return --- .../graphql/GraphQLProtectionInterceptor.java | 29 ++++++++- .../com/predic8/membrane/core/UnitTests.java | 2 + .../GraphQLProtectionInterceptorTest.java | 63 ++++++++++++++++--- 3 files changed, 82 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/graphql/GraphQLProtectionInterceptor.java b/core/src/main/java/com/predic8/membrane/core/graphql/GraphQLProtectionInterceptor.java index 439bc6f15..76130e74e 100644 --- a/core/src/main/java/com/predic8/membrane/core/graphql/GraphQLProtectionInterceptor.java +++ b/core/src/main/java/com/predic8/membrane/core/graphql/GraphQLProtectionInterceptor.java @@ -66,6 +66,7 @@ public class GraphQLProtectionInterceptor extends AbstractInterceptor { private List allowedMethods = Lists.newArrayList("GET", "POST"); private int maxRecursion = 3; private int maxDepth = 7; + private int maxMutations = 5; public GraphQLProtectionInterceptor() { name = "GraphQL protection"; @@ -159,6 +160,9 @@ public Outcome handleRequest(Exchange exc) throws Exception { ExecutableDocument ed = graphQLParser.parseRequest(new ByteArrayInputStream(((String) query).getBytes(UTF_8))); + if (countMutations(ed.getExecutableDefinitions()) > maxMutations) + return error(exc, 400, "Too many mutations defined in document."); + // so far, this ensures uniqueness of global names List e1 = new GraphQLValidator().validate(ed); if (e1 != null && e1.size() > 0) @@ -191,8 +195,6 @@ public Outcome handleRequest(Exchange exc) throws Exception { .map(exd -> (OperationDefinition) exd).toList(); if (ods.size() == 0) return error(exc, "Could not find an OperationDefinition in the GraphQL document."); - if (ods.size() > 1) - return error(exc, "Multiple OperationDefinitions with the same name in the GraphQL document."); operationToExecute = ods.get(0); } @@ -203,6 +205,15 @@ public Outcome handleRequest(Exchange exc) throws Exception { return Outcome.CONTINUE; } + public int countMutations(List definitions) { + return (int) definitions.stream() + .filter(definition -> definition instanceof OperationDefinition) + .map(definition -> (OperationDefinition) definition) + .filter(operation -> operation.getOperationType() != null) + .filter(operation -> operation.getOperationType().getOperation().equals("mutation")) + .count(); + } + private String getDepthOrRecursionError(ExecutableDocument ed, OperationDefinition od) { String err = checkSelections(ed, od, od.getSelections(), new ArrayList<>(), new HashSet<>()); if (err != null) @@ -292,6 +303,20 @@ public boolean isAllowExtensions() { return allowExtensions; } + /** + * Limit how many mutations can be defined in a document query. + * @default 5 + * @example 2 + */ + @MCAttribute + public void setMaxMutations(int maxMutations) { + this.maxMutations = maxMutations; + } + + public int getMaxMutations() { + return maxMutations; + } + /** * Whether to allow GraphQL "extensions". * @default false diff --git a/core/src/test/java/com/predic8/membrane/core/UnitTests.java b/core/src/test/java/com/predic8/membrane/core/UnitTests.java index 6052d6384..6c1c1daa0 100644 --- a/core/src/test/java/com/predic8/membrane/core/UnitTests.java +++ b/core/src/test/java/com/predic8/membrane/core/UnitTests.java @@ -18,6 +18,7 @@ import com.predic8.membrane.core.config.ReadRulesConfigurationTest; import com.predic8.membrane.core.config.ReadRulesWithInterceptorsConfigurationTest; import com.predic8.membrane.core.exchangestore.*; +import com.predic8.membrane.core.graphql.GraphQLProtectionInterceptorTest; import com.predic8.membrane.core.http.*; import com.predic8.membrane.core.http.cookie.*; import com.predic8.membrane.core.interceptor.*; @@ -130,6 +131,7 @@ XMLProtectorTest.class, AbstractExchangeStoreTest.class, JsonProtectionInterceptorTest.class, + GraphQLProtectionInterceptorTest.class, BeautifierInterceptorTest.class, ExchangeEvaluationContextTest.class, PaddingHeaderInterceptorTest.class, diff --git a/core/src/test/java/com/predic8/membrane/core/graphql/GraphQLProtectionInterceptorTest.java b/core/src/test/java/com/predic8/membrane/core/graphql/GraphQLProtectionInterceptorTest.java index 43b6f0212..220c89681 100644 --- a/core/src/test/java/com/predic8/membrane/core/graphql/GraphQLProtectionInterceptorTest.java +++ b/core/src/test/java/com/predic8/membrane/core/graphql/GraphQLProtectionInterceptorTest.java @@ -14,16 +14,23 @@ package com.predic8.membrane.core.graphql; -import com.predic8.membrane.core.*; -import com.predic8.membrane.core.exchange.*; -import com.predic8.membrane.core.http.*; -import com.predic8.membrane.core.interceptor.*; -import org.junit.jupiter.api.*; - -import static com.predic8.membrane.core.http.MimeType.*; -import static java.net.URLEncoder.*; -import static java.nio.charset.StandardCharsets.*; -import static org.junit.jupiter.api.Assertions.*; +import com.predic8.membrane.core.HttpRouter; +import com.predic8.membrane.core.exchange.Exchange; +import com.predic8.membrane.core.graphql.model.*; +import com.predic8.membrane.core.http.Request; +import com.predic8.membrane.core.interceptor.Outcome; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import javax.security.auth.login.Configuration; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import static com.predic8.membrane.core.http.MimeType.APPLICATION_JSON; +import static java.net.URLEncoder.encode; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertEquals; public class GraphQLProtectionInterceptorTest { @@ -339,6 +346,42 @@ public void recursionNotOK() throws Exception { Outcome.RETURN); } + @Test + public void mutationsCountNotOK() throws Exception { + verifyPost("/", + APPLICATION_JSON, + """ + {"query":"mutation abc{} mutation abcd{} mutation abcde{} mutation abcdef{} mutation abcdefg{} mutation abcdefgh{}", + "operationName": ""}""", + Outcome.RETURN); + } + + @Test + public void mutationsCountOK() throws Exception { + verifyPost("/", + APPLICATION_JSON, + """ + {"query":"mutation abc{} mutation abcd{} mutation abcde{} mutation abcdef{} mutation abcdefg{}", + "operationName": ""}""", + Outcome.CONTINUE); + } + + @Test + public void countThreeMutations() { + List definitions = Arrays.asList( + opDefOfType("mutation"), + opDefOfType("mutation"), + opDefOfType("mutation"), + opDefOfType("query") + ); + + assertEquals(3, i.countMutations(definitions)); + } + + private OperationDefinition opDefOfType(String type) { + return new OperationDefinition(new OperationType(type), "", new ArrayList<>(), new ArrayList<>(), new ArrayList<>()); + } + @Test public void invalidMethod() throws Exception { Exchange e = new Request.Builder().put("/").contentType(APPLICATION_JSON).body("{}").buildExchange();