From 2b54b1b2539c9f6ffaf52c967505473982c0db82 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 14 Jun 2024 16:07:30 +0530 Subject: [PATCH 1/8] add depth-limit to runtime engine --- .../Configuration/ConfigurationTests.cs | 97 +++++++++++++++++++ src/Service/Startup.cs | 97 +++++++++++-------- 2 files changed, 155 insertions(+), 39 deletions(-) diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 8ee6d2a3e2..55d548651d 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -3711,6 +3711,82 @@ public async Task ValidateNextLinkUsage() } } + /// + /// Tests the enforcement of depth limit restrictions on GraphQL queries and mutations in non-hosted mode. + /// Verifies that requests exceeding the specified depth limit result in a BadRequest, + /// while requests within the limit succeed with the expected status code. + /// Also verifies that the error message contains the current and allowed max depth limit value. + /// + /// The maximum allowed depth for GraphQL queries and mutations. + /// Indicates whether the operation is a mutation (true) or a query (false). + /// The expected HTTP status code for the operation. + [DataTestMethod] + [DataRow(1, false, HttpStatusCode.BadRequest, DisplayName = "Failed Query execution when max depth limit is set to 1")] + [DataRow(2, false, HttpStatusCode.OK, DisplayName = "Query execution succesful when max depth limit is set to 2")] + [DataRow(1, true, HttpStatusCode.BadRequest, DisplayName = "Failed Mutation execution when max depth limit is set to 1")] + [DataRow(2, true, HttpStatusCode.OK, DisplayName = "Mutation execution succesful when max depth limit is set to 2")] + [TestCategory(TestCategory.MSSQL)] + public async Task TestDepthLimitRestictionOnGrahQLInNonHostedMode( + int depthLimit, + bool isMutationOperation, + HttpStatusCode expectedStatusCodeForGraphQL) + { + // Arrange + GraphQLRuntimeOptions graphqlOptions = new(DepthLimit: depthLimit); + graphqlOptions = graphqlOptions with { UserProvidedDepthLimit = true }; + + DataSource dataSource = new(DatabaseType.MSSQL, + GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL), Options: null); + + RuntimeConfig configuration = InitMinimalRuntimeConfig(dataSource, graphqlOptions, restOptions: new()); + const string CUSTOM_CONFIG = "custom-config.json"; + File.WriteAllText(CUSTOM_CONFIG, configuration.ToJson()); + + string[] args = new[] + { + $"--ConfigFileName={CUSTOM_CONFIG}" + }; + + // Act + using (TestServer server = new(Program.CreateWebHostBuilder(args))) + using (HttpClient client = server.CreateClient()) + { + string query; + if (isMutationOperation) + { + // requested mutation operation has depth of 2 + query = GetSimpleGraphQLCreateMutation(); + } + else + { + // requested query operation has depth of 2 + query = GetSimpleReadGraphQLQuery(); + } + + object payload = new { query }; + + HttpRequestMessage graphQLRequest = new(HttpMethod.Post, "/graphql") + { + Content = JsonContent.Create(payload) + }; + + HttpResponseMessage graphQLResponse = await client.SendAsync(graphQLRequest); + + // Assert + Assert.AreEqual(expectedStatusCodeForGraphQL, graphQLResponse.StatusCode); + string body = await graphQLResponse.Content.ReadAsStringAsync(); + if (expectedStatusCodeForGraphQL == HttpStatusCode.OK) + { + Assert.IsFalse(body.Contains("errors")); + } + else + { + Assert.IsTrue(body.Contains("errors")); + Assert.IsTrue(body.Contains($"The GraphQL document has an execution depth of 2 which exceeds the max allowed execution depth of {depthLimit}.")); + } + } + } + /// /// Helper function to write custom configuration file. with minimal REST/GraphQL global settings /// using the supplied entities. @@ -4249,5 +4325,26 @@ private bool HandleException(Exception e) where T : Exception return false; } + + private static string GetSimpleReadGraphQLQuery() + { + return @"{ + book_by_pk(id: 1) { + id, + title, + publisher_id + } + }"; + } + + private static string GetSimpleGraphQLCreateMutation() + { + return @"mutation createbook{ + createbook(item: { title: ""Book #1"", publisher_id: 1234 }) { + title + publisher_id + } + }"; + } } } diff --git a/src/Service/Startup.cs b/src/Service/Startup.cs index 1fbb22a88f..2f2c869dbb 100644 --- a/src/Service/Startup.cs +++ b/src/Service/Startup.cs @@ -25,6 +25,7 @@ using Azure.DataApiBuilder.Service.Exceptions; using Azure.DataApiBuilder.Service.HealthCheck; using HotChocolate.AspNetCore; +using HotChocolate.Execution.Configuration; using HotChocolate.Types; using Microsoft.ApplicationInsights; using Microsoft.ApplicationInsights.Channel; @@ -189,7 +190,15 @@ public void ConfigureServices(IServiceCollection services) services.AddSingleton(); services.AddSingleton(); - AddGraphQLService(services); + int? depthLimit = null; + if (runtimeConfig is not null + && runtimeConfig.Runtime!.GraphQL!.DepthLimit is not null + && runtimeConfig.Runtime!.GraphQL!.DepthLimit > 0) + { + depthLimit = runtimeConfig.Runtime!.GraphQL!.DepthLimit; + } + + AddGraphQLService(services, depthLimit); services.AddFusionCache() .WithOptions(options => { @@ -213,50 +222,60 @@ public void ConfigureServices(IServiceCollection services) /// when determining whether to allow introspection requests to proceed. /// /// Service Collection - private void AddGraphQLService(IServiceCollection services) + private void AddGraphQLService(IServiceCollection services, int? depthLimit = null) { - services.AddGraphQLServer() - .AddHttpRequestInterceptor() - .ConfigureSchema((serviceProvider, schemaBuilder) => - { - GraphQLSchemaCreator graphQLService = serviceProvider.GetRequiredService(); - graphQLService.InitializeSchemaAndResolvers(schemaBuilder); - }) - .AddHttpRequestInterceptor() - .AddAuthorization() - .AllowIntrospection(false) - .AddAuthorizationHandler() - .AddErrorFilter(error => + IRequestExecutorBuilder server = services.AddGraphQLServer() + .AddHttpRequestInterceptor() + .ConfigureSchema((serviceProvider, schemaBuilder) => + { + GraphQLSchemaCreator graphQLService = serviceProvider.GetRequiredService(); + graphQLService.InitializeSchemaAndResolvers(schemaBuilder); + }) + .AddHttpRequestInterceptor() + .AddAuthorization() + .AllowIntrospection(false) + .AddAuthorizationHandler(); + + // Conditionally adds a maximum depth rule to the GraphQL queries/mutation. + // This rule is only added if a positive depth limit is specified, ensuring that the server + // enforces a limit on the depth of incoming GraphQL queries/mutation to prevent extremly deep queries + // that could potentially lead to performance issues. + if (depthLimit.HasValue && depthLimit.Value > 0) + { + server = server.AddMaxExecutionDepthRule(depthLimit.Value); + } + + server.AddErrorFilter(error => + { + if (error.Exception is not null) { - if (error.Exception is not null) - { - _logger.LogError(exception: error.Exception, message: "A GraphQL request execution error occurred."); - return error.WithMessage(error.Exception.Message); - } + _logger.LogError(exception: error.Exception, message: "A GraphQL request execution error occurred."); + return error.WithMessage(error.Exception.Message); + } - if (error.Code is not null) - { - _logger.LogError(message: "Error code: {errorCode}\nError message: {errorMessage}", error.Code, error.Message); - return error.WithMessage(error.Message); - } + if (error.Code is not null) + { + _logger.LogError(message: "Error code: {errorCode}\nError message: {errorMessage}", error.Code, error.Message); + return error.WithMessage(error.Message); + } - return error; - }) - .AddErrorFilter(error => + return error; + }) + .AddErrorFilter(error => + { + if (error.Exception is DataApiBuilderException thrownException) { - if (error.Exception is DataApiBuilderException thrownException) - { - return error.RemoveException() - .RemoveLocations() - .RemovePath() - .WithMessage(thrownException.Message) - .WithCode($"{thrownException.SubStatusCode}"); - } + return error.RemoveException() + .RemoveLocations() + .RemovePath() + .WithMessage(thrownException.Message) + .WithCode($"{thrownException.SubStatusCode}"); + } - return error; - }) - .UseRequest() - .UseDefaultPipeline(); + return error; + }) + .UseRequest() + .UseDefaultPipeline(); } // This method gets called by the runtime. Use this method to configure the HTTP request pipeline. From 8fc164f34de2f9e5b5d4e877b127f8954b894a96 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 14 Jun 2024 16:36:56 +0530 Subject: [PATCH 2/8] adding tests for introspection queries --- .../Configuration/ConfigurationTests.cs | 70 +++++++++++++++++++ src/Service/Startup.cs | 3 +- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 55d548651d..16403e4f99 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -3787,6 +3787,54 @@ public async Task TestDepthLimitRestictionOnGrahQLInNonHostedMode( } } + /// + /// This Test verfyies that depth-limit specified for graphQL do not affect the introspection queries. + /// In this test, we have specified the depth limit as 2 and we are sending introspection query with depth 6. + /// The expected result is that the query should be successful and should not return any errors. + /// + [TestCategory(TestCategory.MSSQL)] + [TestMethod] + public async Task TestGraphQLIntrospectionQueriesAreNotImpactedByDepthLimit() + { + // Arrange + GraphQLRuntimeOptions graphqlOptions = new(DepthLimit: 2); + graphqlOptions = graphqlOptions with { UserProvidedDepthLimit = true }; + + DataSource dataSource = new(DatabaseType.MSSQL, + GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL), Options: null); + + RuntimeConfig configuration = InitMinimalRuntimeConfig(dataSource, graphqlOptions, restOptions: new()); + const string CUSTOM_CONFIG = "custom-config.json"; + File.WriteAllText(CUSTOM_CONFIG, configuration.ToJson()); + + string[] args = new[] + { + $"--ConfigFileName={CUSTOM_CONFIG}" + }; + + // Act + using (TestServer server = new(Program.CreateWebHostBuilder(args))) + using (HttpClient client = server.CreateClient()) + { + // nested depth:6 + string query = GetSimpleGraphQLIntrospectionQuery(); + + object payload = new { query }; + + HttpRequestMessage graphQLRequest = new(HttpMethod.Post, "/graphql") + { + Content = JsonContent.Create(payload) + }; + + HttpResponseMessage graphQLResponse = await client.SendAsync(graphQLRequest); + + // Assert + Assert.AreEqual(HttpStatusCode.OK, graphQLResponse.StatusCode); + string body = await graphQLResponse.Content.ReadAsStringAsync(); + Assert.IsFalse(body.Contains("errors")); + } + } + /// /// Helper function to write custom configuration file. with minimal REST/GraphQL global settings /// using the supplied entities. @@ -4346,5 +4394,27 @@ private static string GetSimpleGraphQLCreateMutation() } }"; } + + private static string GetSimpleGraphQLIntrospectionQuery() + { + return @"{ + __schema { + types { + name + fields { + name + type { + name + kind + ofType { + name + kind + } + } + } + } + } + }"; + } } } diff --git a/src/Service/Startup.cs b/src/Service/Startup.cs index 2f2c869dbb..61e86b53ee 100644 --- a/src/Service/Startup.cs +++ b/src/Service/Startup.cs @@ -240,9 +240,10 @@ private void AddGraphQLService(IServiceCollection services, int? depthLimit = nu // This rule is only added if a positive depth limit is specified, ensuring that the server // enforces a limit on the depth of incoming GraphQL queries/mutation to prevent extremly deep queries // that could potentially lead to performance issues. + // Additionally, the skipIntrospectionFields parameter is set to true to prevent the depth limit rule for introspection queries. if (depthLimit.HasValue && depthLimit.Value > 0) { - server = server.AddMaxExecutionDepthRule(depthLimit.Value); + server = server.AddMaxExecutionDepthRule(maxAllowedExecutionDepth: depthLimit.Value, skipIntrospectionFields: true); } server.AddErrorFilter(error => From dad749f942f1f3e081715391201c15258a43d6a3 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 17 Jun 2024 18:33:57 +0530 Subject: [PATCH 3/8] adding more tests --- schemas/dab.draft.schema.json | 2 +- .../GraphQLRuntimeOptionsConverterFactory.cs | 8 +- .../Configuration/ConfigurationTests.cs | 104 ++++++++++++++++++ 3 files changed, 112 insertions(+), 2 deletions(-) diff --git a/schemas/dab.draft.schema.json b/schemas/dab.draft.schema.json index 69d4e3212c..a764d064c4 100644 --- a/schemas/dab.draft.schema.json +++ b/schemas/dab.draft.schema.json @@ -177,7 +177,7 @@ "description": "Allow enabling/disabling GraphQL requests for all entities." }, "depth-limit": { - "type": "integer", + "type": [ "integer", "null" ], "description": "Maximum allowed depth of a GraphQL query.", "default": null }, diff --git a/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs b/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs index ecd3252bb9..8b3984fd07 100644 --- a/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs +++ b/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs @@ -132,7 +132,13 @@ internal GraphQLRuntimeOptionsConverter(bool replaceEnvVar) } else if (reader.TokenType is JsonTokenType.Number) { - graphQLRuntimeOptions = graphQLRuntimeOptions with { DepthLimit = reader.GetInt32(), UserProvidedDepthLimit = true }; + int depthLimit = reader.GetInt32(); + if (depthLimit < -1 || depthLimit == 0) + { + throw new JsonException($"Invalid depth-limit: {depthLimit}. Specify a depth limit > 0 or remove the existing depth limit by specifying -1."); + } + + graphQLRuntimeOptions = graphQLRuntimeOptions with { DepthLimit = depthLimit, UserProvidedDepthLimit = true }; } else { diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 16403e4f99..40208e379f 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -1299,6 +1299,58 @@ public async Task TestConfigIsValid() } } + /// + /// Test to verify that provided value of depth-limit in the config file should be greater than 0. + /// -1 and null are special values to disable depth limit. + /// This test validates that depth-limit outside the valid range should fail validation. + /// + [DataTestMethod] + [DataRow(-1, DisplayName = "[PASS]: Valid Value: -1 to disable depth limit")] + [DataRow(0, DisplayName = "[FAIL]: Invalid Value: 0 for depth-limit.")] + [DataRow(2, DisplayName = "[PASS]: Valid Value: 2 for depth-limit.")] + [DataRow(null, DisplayName = "[PASS]: Valid Value: null for depth-limit.")] + [TestCategory(TestCategory.MSSQL)] + public async Task TestValidateConfigForDifferentDepthLimit(int? depthLimit) + { + // Fetch the MS_SQL integration test config file. + TestHelper.SetupDatabaseEnvironment(MSSQL_ENVIRONMENT); + FileSystemRuntimeConfigLoader testConfigPath = TestHelper.GetRuntimeConfigLoader(); + RuntimeConfig configuration = TestHelper.GetRuntimeConfigProvider(testConfigPath).GetConfig(); + configuration = configuration with + { + Runtime = configuration.Runtime with + { + GraphQL = configuration.Runtime.GraphQL with { DepthLimit = depthLimit, UserProvidedDepthLimit = true } + } + }; + const string CUSTOM_CONFIG = "custom-config.json"; + + MockFileSystem fileSystem = new(); + + // write it to the custom-config file and add it to the filesystem. + fileSystem.AddFile(CUSTOM_CONFIG, new MockFileData(configuration.ToJson())); + FileSystemRuntimeConfigLoader configLoader = new(fileSystem); + configLoader.UpdateConfigFilePath(CUSTOM_CONFIG); + RuntimeConfigProvider configProvider = TestHelper.GetRuntimeConfigProvider(configLoader); + + Mock> configValidatorLogger = new(); + RuntimeConfigValidator configValidator = + new( + configProvider, + fileSystem, + configValidatorLogger.Object, + true); + + if (depthLimit is not null && depthLimit <= 0 && depthLimit != -1) + { + Assert.IsFalse(await configValidator.TryValidateConfig(CUSTOM_CONFIG, TestHelper.ProvisionLoggerFactory())); + } + else + { + Assert.IsTrue(await configValidator.TryValidateConfig(CUSTOM_CONFIG, TestHelper.ProvisionLoggerFactory())); + } + } + /// /// This test method checks a valid config's entities against /// the database and ensures they are valid. @@ -3835,6 +3887,58 @@ public async Task TestGraphQLIntrospectionQueriesAreNotImpactedByDepthLimit() } } + /// + /// Tests the behavior of GraphQL queries in non-hosted mode when the depth limit is explicitly set to -1 or null. + /// Setting the depth limit to -1 or null is intended to disable the depth limit check, allowing queries of any depth. + /// This test verifies that queries are processed successfully without any errors under these configurations. + /// + /// + [DataTestMethod] + [DataRow(-1, DisplayName = "Setting -1 for depth-limit will disable the depth limit")] + [DataRow(null, DisplayName = "Setting null for depth-limit will disable the depth limit")] + [TestCategory(TestCategory.MSSQL)] + public async Task TestNoDepthLimitOnGrahQLInNonHostedMode( + int? depthLimit) + { + // Arrange + GraphQLRuntimeOptions graphqlOptions = new(DepthLimit: depthLimit); + graphqlOptions = graphqlOptions with { UserProvidedDepthLimit = true }; + + DataSource dataSource = new(DatabaseType.MSSQL, + GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL), Options: null); + + RuntimeConfig configuration = InitMinimalRuntimeConfig(dataSource, graphqlOptions, restOptions: new()); + const string CUSTOM_CONFIG = "custom-config.json"; + File.WriteAllText(CUSTOM_CONFIG, configuration.ToJson()); + + string[] args = new[] + { + $"--ConfigFileName={CUSTOM_CONFIG}" + }; + + // Act + using (TestServer server = new(Program.CreateWebHostBuilder(args))) + using (HttpClient client = server.CreateClient()) + { + // requested query operation has depth of 2 + string query = GetSimpleReadGraphQLQuery(); + + object payload = new { query }; + + HttpRequestMessage graphQLRequest = new(HttpMethod.Post, "/graphql") + { + Content = JsonContent.Create(payload) + }; + + HttpResponseMessage graphQLResponse = await client.SendAsync(graphQLRequest); + + // Assert + Assert.AreEqual(HttpStatusCode.OK, graphQLResponse.StatusCode); + string body = await graphQLResponse.Content.ReadAsStringAsync(); + Assert.IsFalse(body.Contains("errors")); + } + } + /// /// Helper function to write custom configuration file. with minimal REST/GraphQL global settings /// using the supplied entities. From 362de95114c33285881e69f863f795e9f49e0d40 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 18 Jun 2024 15:43:05 +0530 Subject: [PATCH 4/8] fix formatting adding more test cases --- .../GraphQLRuntimeOptionsConverterFactory.cs | 11 +- .../Configuration/ConfigurationTests.cs | 128 ++++++++---------- src/Service/Startup.cs | 6 +- 3 files changed, 73 insertions(+), 72 deletions(-) diff --git a/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs b/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs index 8b3984fd07..c891651e7f 100644 --- a/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs +++ b/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs @@ -132,7 +132,16 @@ internal GraphQLRuntimeOptionsConverter(bool replaceEnvVar) } else if (reader.TokenType is JsonTokenType.Number) { - int depthLimit = reader.GetInt32(); + int depthLimit; + try + { + depthLimit = reader.GetInt32(); + } + catch (JsonException) + { + throw new JsonException($"The 'depth-limit' value must be an integer within the valid range of 1 to Int32.Max or -1."); + } + if (depthLimit < -1 || depthLimit == 0) { throw new JsonException($"Invalid depth-limit: {depthLimit}. Specify a depth limit > 0 or remove the existing depth limit by specifying -1."); diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 40208e379f..d9216680ac 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -1302,18 +1302,23 @@ public async Task TestConfigIsValid() /// /// Test to verify that provided value of depth-limit in the config file should be greater than 0. /// -1 and null are special values to disable depth limit. - /// This test validates that depth-limit outside the valid range should fail validation. + /// This test validates that depth-limit outside the valid range should fail validation + /// during `dab validate` and `dab start`. /// [DataTestMethod] - [DataRow(-1, DisplayName = "[PASS]: Valid Value: -1 to disable depth limit")] - [DataRow(0, DisplayName = "[FAIL]: Invalid Value: 0 for depth-limit.")] - [DataRow(2, DisplayName = "[PASS]: Valid Value: 2 for depth-limit.")] - [DataRow(null, DisplayName = "[PASS]: Valid Value: null for depth-limit.")] + [DataRow(-1, true, DisplayName = "[PASS]: Valid Value: -1 to disable depth limit")] + [DataRow(0, false, DisplayName = "[FAIL]: Invalid Value: 0 for depth-limit.")] + [DataRow(-2, false, DisplayName = "[FAIL]: Invalid Value: -2 for depth-limit.")] + [DataRow(2, true, DisplayName = "[PASS]: Valid Value: 2 for depth-limit.")] + [DataRow(2147483647, true, DisplayName = "[PASS]: Valid Value: Using Int32.MaxValue(2147483647) for depth-limit.")] + [DataRow(null, true, DisplayName = "[PASS]: Valid Value: null for depth-limit.")] [TestCategory(TestCategory.MSSQL)] - public async Task TestValidateConfigForDifferentDepthLimit(int? depthLimit) + public async Task TestValidateConfigForDifferentDepthLimit(int? depthLimit, bool isValidInput) { + //Arrange // Fetch the MS_SQL integration test config file. TestHelper.SetupDatabaseEnvironment(MSSQL_ENVIRONMENT); + const string CUSTOM_CONFIG = "custom-config.json"; FileSystemRuntimeConfigLoader testConfigPath = TestHelper.GetRuntimeConfigLoader(); RuntimeConfig configuration = TestHelper.GetRuntimeConfigProvider(testConfigPath).GetConfig(); configuration = configuration with @@ -1323,11 +1328,10 @@ public async Task TestValidateConfigForDifferentDepthLimit(int? depthLimit) GraphQL = configuration.Runtime.GraphQL with { DepthLimit = depthLimit, UserProvidedDepthLimit = true } } }; - const string CUSTOM_CONFIG = "custom-config.json"; MockFileSystem fileSystem = new(); - // write it to the custom-config file and add it to the filesystem. + // write the content of configuration to the custom-config file and add it to the filesystem. fileSystem.AddFile(CUSTOM_CONFIG, new MockFileData(configuration.ToJson())); FileSystemRuntimeConfigLoader configLoader = new(fileSystem); configLoader.UpdateConfigFilePath(CUSTOM_CONFIG); @@ -1341,14 +1345,11 @@ public async Task TestValidateConfigForDifferentDepthLimit(int? depthLimit) configValidatorLogger.Object, true); - if (depthLimit is not null && depthLimit <= 0 && depthLimit != -1) - { - Assert.IsFalse(await configValidator.TryValidateConfig(CUSTOM_CONFIG, TestHelper.ProvisionLoggerFactory())); - } - else - { - Assert.IsTrue(await configValidator.TryValidateConfig(CUSTOM_CONFIG, TestHelper.ProvisionLoggerFactory())); - } + // Act + bool isSuccess = await configValidator.TryValidateConfig(CUSTOM_CONFIG, TestHelper.ProvisionLoggerFactory()); + + // Assert + Assert.AreEqual(isValidInput, isSuccess); } /// @@ -3774,11 +3775,11 @@ public async Task ValidateNextLinkUsage() /// The expected HTTP status code for the operation. [DataTestMethod] [DataRow(1, false, HttpStatusCode.BadRequest, DisplayName = "Failed Query execution when max depth limit is set to 1")] - [DataRow(2, false, HttpStatusCode.OK, DisplayName = "Query execution succesful when max depth limit is set to 2")] + [DataRow(2, false, HttpStatusCode.OK, DisplayName = "Query execution successful when max depth limit is set to 2")] [DataRow(1, true, HttpStatusCode.BadRequest, DisplayName = "Failed Mutation execution when max depth limit is set to 1")] - [DataRow(2, true, HttpStatusCode.OK, DisplayName = "Mutation execution succesful when max depth limit is set to 2")] + [DataRow(2, true, HttpStatusCode.OK, DisplayName = "Mutation execution successful when max depth limit is set to 2")] [TestCategory(TestCategory.MSSQL)] - public async Task TestDepthLimitRestictionOnGrahQLInNonHostedMode( + public async Task TestDepthLimitRestrictionOnGraphQLInNonHostedMode( int depthLimit, bool isMutationOperation, HttpStatusCode expectedStatusCodeForGraphQL) @@ -3799,7 +3800,6 @@ public async Task TestDepthLimitRestictionOnGrahQLInNonHostedMode( $"--ConfigFileName={CUSTOM_CONFIG}" }; - // Act using (TestServer server = new(Program.CreateWebHostBuilder(args))) using (HttpClient client = server.CreateClient()) { @@ -3807,12 +3807,23 @@ public async Task TestDepthLimitRestictionOnGrahQLInNonHostedMode( if (isMutationOperation) { // requested mutation operation has depth of 2 - query = GetSimpleGraphQLCreateMutation(); + query = @"mutation createbook{ + createbook(item: { title: ""Book #1"", publisher_id: 1234 }) { // depth: 1 + title // depth: 2 + publisher_id + } + }"; } else { // requested query operation has depth of 2 - query = GetSimpleReadGraphQLQuery(); + query = @"{ + book_by_pk(id: 1) { // depth: 1 + id, // depth: 2 + title, + publisher_id + } + }"; } object payload = new { query }; @@ -3822,12 +3833,13 @@ public async Task TestDepthLimitRestictionOnGrahQLInNonHostedMode( Content = JsonContent.Create(payload) }; + // Act HttpResponseMessage graphQLResponse = await client.SendAsync(graphQLRequest); // Assert Assert.AreEqual(expectedStatusCodeForGraphQL, graphQLResponse.StatusCode); string body = await graphQLResponse.Content.ReadAsStringAsync(); - if (expectedStatusCodeForGraphQL == HttpStatusCode.OK) + if (graphQLResponse.StatusCode == HttpStatusCode.OK) { Assert.IsFalse(body.Contains("errors")); } @@ -3869,7 +3881,24 @@ public async Task TestGraphQLIntrospectionQueriesAreNotImpactedByDepthLimit() using (HttpClient client = server.CreateClient()) { // nested depth:6 - string query = GetSimpleGraphQLIntrospectionQuery(); + string query = @"{ + __schema { // depth: 1 + types { // depth: 2 + name + fields { + name // depth: 3 + type { + name // depth: 4 + kind + ofType { // depth: 5 + name // depth: 6 + kind + } + } + } + } + } + }"; object payload = new { query }; @@ -3921,7 +3950,13 @@ public async Task TestNoDepthLimitOnGrahQLInNonHostedMode( using (HttpClient client = server.CreateClient()) { // requested query operation has depth of 2 - string query = GetSimpleReadGraphQLQuery(); + string query = @"{ + book_by_pk(id: 1) { // depth: 1 + id, // depth: 2 + title, + publisher_id + } + }"; object payload = new { query }; @@ -4477,48 +4512,5 @@ private bool HandleException(Exception e) where T : Exception return false; } - - private static string GetSimpleReadGraphQLQuery() - { - return @"{ - book_by_pk(id: 1) { - id, - title, - publisher_id - } - }"; - } - - private static string GetSimpleGraphQLCreateMutation() - { - return @"mutation createbook{ - createbook(item: { title: ""Book #1"", publisher_id: 1234 }) { - title - publisher_id - } - }"; - } - - private static string GetSimpleGraphQLIntrospectionQuery() - { - return @"{ - __schema { - types { - name - fields { - name - type { - name - kind - ofType { - name - kind - } - } - } - } - } - }"; - } } } diff --git a/src/Service/Startup.cs b/src/Service/Startup.cs index 61e86b53ee..d12b444b52 100644 --- a/src/Service/Startup.cs +++ b/src/Service/Startup.cs @@ -236,11 +236,11 @@ private void AddGraphQLService(IServiceCollection services, int? depthLimit = nu .AllowIntrospection(false) .AddAuthorizationHandler(); - // Conditionally adds a maximum depth rule to the GraphQL queries/mutation. + // Conditionally adds a maximum depth rule to the GraphQL queries/mutation selection set. // This rule is only added if a positive depth limit is specified, ensuring that the server - // enforces a limit on the depth of incoming GraphQL queries/mutation to prevent extremly deep queries + // enforces a limit on the depth of incoming GraphQL queries/mutation to prevent extremely deep queries // that could potentially lead to performance issues. - // Additionally, the skipIntrospectionFields parameter is set to true to prevent the depth limit rule for introspection queries. + // Additionally, the skipIntrospectionFields parameter is set to true to skip depth limit enforcement on introspection queries. if (depthLimit.HasValue && depthLimit.Value > 0) { server = server.AddMaxExecutionDepthRule(maxAllowedExecutionDepth: depthLimit.Value, skipIntrospectionFields: true); From 17c739563736ce27936ccf55136f5288a9a87bf5 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 19 Jun 2024 16:04:56 +0530 Subject: [PATCH 5/8] updating tests --- .../GraphQLRuntimeOptionsConverterFactory.cs | 4 +- .../Configuration/ConfigurationTests.cs | 111 +++++++++++++----- src/Service/Startup.cs | 16 +-- 3 files changed, 90 insertions(+), 41 deletions(-) diff --git a/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs b/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs index c891651e7f..9251169cd7 100644 --- a/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs +++ b/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs @@ -137,9 +137,9 @@ internal GraphQLRuntimeOptionsConverter(bool replaceEnvVar) { depthLimit = reader.GetInt32(); } - catch (JsonException) + catch (FormatException e) { - throw new JsonException($"The 'depth-limit' value must be an integer within the valid range of 1 to Int32.Max or -1."); + throw new JsonException($"Failed to parse the depth-limit value: {e.Message}"); } if (depthLimit < -1 || depthLimit == 0) diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index d9216680ac..d70eb37afc 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -1301,7 +1301,8 @@ public async Task TestConfigIsValid() /// /// Test to verify that provided value of depth-limit in the config file should be greater than 0. - /// -1 and null are special values to disable depth limit. + /// -1 and null are special values. + /// -1 can be set to remove the depth limit, while `null` is the default value which means no depth limit check. /// This test validates that depth-limit outside the valid range should fail validation /// during `dab validate` and `dab start`. /// @@ -1311,7 +1312,7 @@ public async Task TestConfigIsValid() [DataRow(-2, false, DisplayName = "[FAIL]: Invalid Value: -2 for depth-limit.")] [DataRow(2, true, DisplayName = "[PASS]: Valid Value: 2 for depth-limit.")] [DataRow(2147483647, true, DisplayName = "[PASS]: Valid Value: Using Int32.MaxValue(2147483647) for depth-limit.")] - [DataRow(null, true, DisplayName = "[PASS]: Valid Value: null for depth-limit.")] + [DataRow(null, true, DisplayName = "[PASS]: Default Value: null for depth-limit.")] [TestCategory(TestCategory.MSSQL)] public async Task TestValidateConfigForDifferentDepthLimit(int? depthLimit, bool isValidInput) { @@ -3769,6 +3770,21 @@ public async Task ValidateNextLinkUsage() /// Verifies that requests exceeding the specified depth limit result in a BadRequest, /// while requests within the limit succeed with the expected status code. /// Also verifies that the error message contains the current and allowed max depth limit value. + /// Example: + /// Query: + /// query book_by_pk{ + /// book_by_pk(id: 1) { // depth: 1 + /// id, // depth: 2 + /// title, // depth: 2 + /// publisher_id // depth: 2 + /// } + /// } + /// Mutation: + /// mutation createbook { + /// createbook(item: { title: ""Book #1"", publisher_id: 1234 }) { // depth: 1 + /// title, // depth: 2 + /// publisher_id // depth: 2 + /// } /// /// The maximum allowed depth for GraphQL queries and mutations. /// Indicates whether the operation is a mutation (true) or a query (false). @@ -3808,8 +3824,8 @@ public async Task TestDepthLimitRestrictionOnGraphQLInNonHostedMode( { // requested mutation operation has depth of 2 query = @"mutation createbook{ - createbook(item: { title: ""Book #1"", publisher_id: 1234 }) { // depth: 1 - title // depth: 2 + createbook(item: { title: ""Book #1"", publisher_id: 1234 }) { + title publisher_id } }"; @@ -3817,9 +3833,9 @@ public async Task TestDepthLimitRestrictionOnGraphQLInNonHostedMode( else { // requested query operation has depth of 2 - query = @"{ - book_by_pk(id: 1) { // depth: 1 - id, // depth: 2 + query = @"query book_by_pk{ + book_by_pk(id: 1) { + id, title, publisher_id } @@ -3839,22 +3855,44 @@ public async Task TestDepthLimitRestrictionOnGraphQLInNonHostedMode( // Assert Assert.AreEqual(expectedStatusCodeForGraphQL, graphQLResponse.StatusCode); string body = await graphQLResponse.Content.ReadAsStringAsync(); + JsonElement responseJson = JsonSerializer.Deserialize(body); if (graphQLResponse.StatusCode == HttpStatusCode.OK) { - Assert.IsFalse(body.Contains("errors")); + Assert.IsFalse(body.Contains("errors"), "The response should not contain any errors."); + Assert.IsNotNull(responseJson.GetProperty("data"), "The response should contain data."); + } else { - Assert.IsTrue(body.Contains("errors")); - Assert.IsTrue(body.Contains($"The GraphQL document has an execution depth of 2 which exceeds the max allowed execution depth of {depthLimit}.")); + Assert.IsNotNull(responseJson.GetProperty("errors"), "The response should contain errors."); + string errorMessage = responseJson.GetProperty("errors").EnumerateArray().FirstOrDefault().GetProperty("message").GetString(); + string expectedErrorMessage = $"The GraphQL document has an execution depth of 2 which exceeds the max allowed execution depth of {depthLimit}."; + Assert.AreEqual(expectedErrorMessage, errorMessage, "The error message should contain the current and allowed max depth limit value."); } } } /// - /// This Test verfyies that depth-limit specified for graphQL do not affect the introspection queries. + /// This test verifies that the depth-limit specified for GraphQL does not affect introspection queries. /// In this test, we have specified the depth limit as 2 and we are sending introspection query with depth 6. /// The expected result is that the query should be successful and should not return any errors. + /// Example: + /// { + /// __schema { // depth: 1 + /// types { // depth: 2 + /// name // depth: 3 + /// fields { // depth: 3 + /// name // depth: 4 + /// type { // depth: 4 + /// name // depth: 5 + /// kind // depth: 5 + /// ofType { // depth: 5 + /// name // depth: 6 + /// kind // depth: 6 + /// } + /// } + /// } + /// } /// [TestCategory(TestCategory.MSSQL)] [TestMethod] @@ -3876,22 +3914,21 @@ public async Task TestGraphQLIntrospectionQueriesAreNotImpactedByDepthLimit() $"--ConfigFileName={CUSTOM_CONFIG}" }; - // Act using (TestServer server = new(Program.CreateWebHostBuilder(args))) using (HttpClient client = server.CreateClient()) { // nested depth:6 string query = @"{ - __schema { // depth: 1 - types { // depth: 2 + __schema { + types { name fields { - name // depth: 3 + name type { - name // depth: 4 + name kind - ofType { // depth: 5 - name // depth: 6 + ofType { + name kind } } @@ -3907,27 +3944,43 @@ public async Task TestGraphQLIntrospectionQueriesAreNotImpactedByDepthLimit() Content = JsonContent.Create(payload) }; + // Act HttpResponseMessage graphQLResponse = await client.SendAsync(graphQLRequest); // Assert Assert.AreEqual(HttpStatusCode.OK, graphQLResponse.StatusCode); string body = await graphQLResponse.Content.ReadAsStringAsync(); - Assert.IsFalse(body.Contains("errors")); + Assert.IsFalse(body.Contains("errors"), "The response should not contain any errors."); + + JsonElement responseJson = JsonSerializer.Deserialize(body); + Assert.IsNotNull(responseJson, "The response should be a valid JSON."); + responseJson.GetProperty("data").GetProperty("__schema").GetProperty("types"); + + JsonElement schema = responseJson.GetProperty("data").GetProperty("__schema"); + Assert.IsNotNull(schema, "The response should contain schema information."); } } /// /// Tests the behavior of GraphQL queries in non-hosted mode when the depth limit is explicitly set to -1 or null. - /// Setting the depth limit to -1 or null is intended to disable the depth limit check, allowing queries of any depth. + /// Setting the depth limit to -1 is intended to disable the depth limit check, allowing queries of any depth. + /// Using null as default value of dab which also disables the depth limit check. /// This test verifies that queries are processed successfully without any errors under these configurations. + /// Example Query: + /// { + /// book_by_pk(id: 1) { // depth: 1 + /// id, // depth: 2 + /// title, // depth: 2 + /// publisher_id // depth: 2 + /// } + /// } /// /// [DataTestMethod] [DataRow(-1, DisplayName = "Setting -1 for depth-limit will disable the depth limit")] - [DataRow(null, DisplayName = "Setting null for depth-limit will disable the depth limit")] + [DataRow(null, DisplayName = "Using default value: null for depth-limit which also disables the depth limit check")] [TestCategory(TestCategory.MSSQL)] - public async Task TestNoDepthLimitOnGrahQLInNonHostedMode( - int? depthLimit) + public async Task TestNoDepthLimitOnGrahQLInNonHostedMode(int? depthLimit) { // Arrange GraphQLRuntimeOptions graphqlOptions = new(DepthLimit: depthLimit); @@ -3945,14 +3998,13 @@ public async Task TestNoDepthLimitOnGrahQLInNonHostedMode( $"--ConfigFileName={CUSTOM_CONFIG}" }; - // Act using (TestServer server = new(Program.CreateWebHostBuilder(args))) using (HttpClient client = server.CreateClient()) { // requested query operation has depth of 2 string query = @"{ - book_by_pk(id: 1) { // depth: 1 - id, // depth: 2 + book_by_pk(id: 1) { + id, title, publisher_id } @@ -3965,12 +4017,17 @@ public async Task TestNoDepthLimitOnGrahQLInNonHostedMode( Content = JsonContent.Create(payload) }; + // Act HttpResponseMessage graphQLResponse = await client.SendAsync(graphQLRequest); // Assert Assert.AreEqual(HttpStatusCode.OK, graphQLResponse.StatusCode); string body = await graphQLResponse.Content.ReadAsStringAsync(); - Assert.IsFalse(body.Contains("errors")); + Assert.IsFalse(body.Contains("errors"), "The response should not contain any errors."); + + JsonElement responseJson = JsonSerializer.Deserialize(body); + Assert.IsNotNull(responseJson.GetProperty("data"), "The response should contain data."); + Assert.IsNotNull(responseJson.GetProperty("data").GetProperty("book_by_pk"), "The response should contain book_by_pk data."); } } diff --git a/src/Service/Startup.cs b/src/Service/Startup.cs index d12b444b52..77ebb63b9d 100644 --- a/src/Service/Startup.cs +++ b/src/Service/Startup.cs @@ -190,15 +190,7 @@ public void ConfigureServices(IServiceCollection services) services.AddSingleton(); services.AddSingleton(); - int? depthLimit = null; - if (runtimeConfig is not null - && runtimeConfig.Runtime!.GraphQL!.DepthLimit is not null - && runtimeConfig.Runtime!.GraphQL!.DepthLimit > 0) - { - depthLimit = runtimeConfig.Runtime!.GraphQL!.DepthLimit; - } - - AddGraphQLService(services, depthLimit); + AddGraphQLService(services, runtimeConfig?.Runtime?.GraphQL); services.AddFusionCache() .WithOptions(options => { @@ -222,7 +214,7 @@ public void ConfigureServices(IServiceCollection services) /// when determining whether to allow introspection requests to proceed. /// /// Service Collection - private void AddGraphQLService(IServiceCollection services, int? depthLimit = null) + private void AddGraphQLService(IServiceCollection services, GraphQLRuntimeOptions? graphQLRuntimeOptions) { IRequestExecutorBuilder server = services.AddGraphQLServer() .AddHttpRequestInterceptor() @@ -241,9 +233,9 @@ private void AddGraphQLService(IServiceCollection services, int? depthLimit = nu // enforces a limit on the depth of incoming GraphQL queries/mutation to prevent extremely deep queries // that could potentially lead to performance issues. // Additionally, the skipIntrospectionFields parameter is set to true to skip depth limit enforcement on introspection queries. - if (depthLimit.HasValue && depthLimit.Value > 0) + if (graphQLRuntimeOptions is not null && graphQLRuntimeOptions.DepthLimit.HasValue && graphQLRuntimeOptions.DepthLimit.Value > 0) { - server = server.AddMaxExecutionDepthRule(maxAllowedExecutionDepth: depthLimit.Value, skipIntrospectionFields: true); + server = server.AddMaxExecutionDepthRule(maxAllowedExecutionDepth: graphQLRuntimeOptions.DepthLimit.Value, skipIntrospectionFields: true); } server.AddErrorFilter(error => From 93007f69d61ea4995155f4a46eda50d04a8697fd Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 21 Jun 2024 21:55:53 +0530 Subject: [PATCH 6/8] fixing nits --- .../GraphQLRuntimeOptionsConverterFactory.cs | 4 +-- .../Configuration/ConfigurationTests.cs | 25 ++++++++++--------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs b/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs index 9251169cd7..1c3fde09f4 100644 --- a/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs +++ b/src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs @@ -137,9 +137,9 @@ internal GraphQLRuntimeOptionsConverter(bool replaceEnvVar) { depthLimit = reader.GetInt32(); } - catch (FormatException e) + catch (FormatException) { - throw new JsonException($"Failed to parse the depth-limit value: {e.Message}"); + throw new JsonException($"The JSON token value is of the incorrect numeric format."); } if (depthLimit < -1 || depthLimit == 0) diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index d70eb37afc..8224307a47 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -3858,14 +3858,15 @@ public async Task TestDepthLimitRestrictionOnGraphQLInNonHostedMode( JsonElement responseJson = JsonSerializer.Deserialize(body); if (graphQLResponse.StatusCode == HttpStatusCode.OK) { - Assert.IsFalse(body.Contains("errors"), "The response should not contain any errors."); - Assert.IsNotNull(responseJson.GetProperty("data"), "The response should contain data."); - + Assert.IsTrue(responseJson.TryGetProperty("data", out JsonElement data), "The response should contain data."); + Assert.IsFalse(data.TryGetProperty("errors", out _), "The response should not contain any errors."); } else { - Assert.IsNotNull(responseJson.GetProperty("errors"), "The response should contain errors."); - string errorMessage = responseJson.GetProperty("errors").EnumerateArray().FirstOrDefault().GetProperty("message").GetString(); + Assert.IsTrue(responseJson.TryGetProperty("errors", out JsonElement data), "The response should contain errors."); + Assert.IsTrue(data.EnumerateArray().Any(), "The response should contain at least one error."); + Assert.IsTrue(data.EnumerateArray().FirstOrDefault().TryGetProperty("message", out JsonElement message), "The error should contain a message.")); + string errorMessage = message.GetString(); string expectedErrorMessage = $"The GraphQL document has an execution depth of 2 which exceeds the max allowed execution depth of {depthLimit}."; Assert.AreEqual(expectedErrorMessage, errorMessage, "The error message should contain the current and allowed max depth limit value."); } @@ -3950,13 +3951,12 @@ public async Task TestGraphQLIntrospectionQueriesAreNotImpactedByDepthLimit() // Assert Assert.AreEqual(HttpStatusCode.OK, graphQLResponse.StatusCode); string body = await graphQLResponse.Content.ReadAsStringAsync(); - Assert.IsFalse(body.Contains("errors"), "The response should not contain any errors."); JsonElement responseJson = JsonSerializer.Deserialize(body); Assert.IsNotNull(responseJson, "The response should be a valid JSON."); - responseJson.GetProperty("data").GetProperty("__schema").GetProperty("types"); - - JsonElement schema = responseJson.GetProperty("data").GetProperty("__schema"); + Assert.IsTrue(responseJson.TryGetProperty("data", out JsonElement data), "The response should contain data."); + Assert.IsFalse(data.TryGetProperty("errors", out _), "The response should not contain any errors."); + Assert.IsTrue(responseJson.GetProperty("data").TryGetProperty("__schema", out JsonElement schema)); Assert.IsNotNull(schema, "The response should contain schema information."); } } @@ -4023,11 +4023,12 @@ public async Task TestNoDepthLimitOnGrahQLInNonHostedMode(int? depthLimit) // Assert Assert.AreEqual(HttpStatusCode.OK, graphQLResponse.StatusCode); string body = await graphQLResponse.Content.ReadAsStringAsync(); - Assert.IsFalse(body.Contains("errors"), "The response should not contain any errors."); JsonElement responseJson = JsonSerializer.Deserialize(body); - Assert.IsNotNull(responseJson.GetProperty("data"), "The response should contain data."); - Assert.IsNotNull(responseJson.GetProperty("data").GetProperty("book_by_pk"), "The response should contain book_by_pk data."); + Assert.IsNotNull(responseJson, "The response should be a valid JSON."); + Assert.IsTrue(responseJson.TryGetProperty("data", out JsonElement data), "The response should contain data."); + Assert.IsFalse(data.TryGetProperty("errors", out _), "The response should not contain any errors."); + Assert.IsTrue(data.TryGetProperty("book_by_pk", out _), "The response data should contain book_by_pk data."); } } From 55ae76c655b5d37c5084e15477b5458ed43fa7e3 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 24 Jun 2024 01:13:39 +0530 Subject: [PATCH 7/8] fixing nits --- src/Service.Tests/Configuration/ConfigurationTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 65d10683d8..fd593b0442 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -3852,7 +3852,7 @@ public async Task TestDepthLimitRestrictionOnGraphQLInNonHostedMode( { Assert.IsTrue(responseJson.TryGetProperty("errors", out JsonElement data), "The response should contain errors."); Assert.IsTrue(data.EnumerateArray().Any(), "The response should contain at least one error."); - Assert.IsTrue(data.EnumerateArray().FirstOrDefault().TryGetProperty("message", out JsonElement message), "The error should contain a message.")); + Assert.IsTrue(data.EnumerateArray().FirstOrDefault().TryGetProperty("message", out JsonElement message), "The error should contain a message."); string errorMessage = message.GetString(); string expectedErrorMessage = $"The GraphQL document has an execution depth of 2 which exceeds the max allowed execution depth of {depthLimit}."; Assert.AreEqual(expectedErrorMessage, errorMessage, "The error message should contain the current and allowed max depth limit value."); From 62dce25a3a8c1aaac36b527ff1d06c532fb521ae Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 1 Jul 2024 10:34:18 +0530 Subject: [PATCH 8/8] fixing nits in tests --- .../Configuration/ConfigurationTests.cs | 71 +++++++++++-------- 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index fd593b0442..bbd07e28ce 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -1299,24 +1299,44 @@ public async Task TestConfigIsValid() } /// - /// Test to verify that provided value of depth-limit in the config file should be greater than 0. + /// Test to verify that provided invalid value of depth-limit in the config file should + /// result in validation failure during `dab validate` and `dab start`. + /// + [DataTestMethod] + [DataRow(0, DisplayName = "[FAIL]: Invalid Value: 0 for depth-limit.")] + [DataRow(-2, DisplayName = "[FAIL]: Invalid Value: -2 for depth-limit.")] + [TestCategory(TestCategory.MSSQL)] + public async Task TestValidateConfigForInvalidDepthLimit(int? depthLimit) + { + await ValidateConfigWithDepthLimit(depthLimit, expectedSuccess: false); + } + + /// + /// Test to verify that provided valid value of depth-limit in the config file should not + /// result in any validation failure during `dab validate` and `dab start`. /// -1 and null are special values. /// -1 can be set to remove the depth limit, while `null` is the default value which means no depth limit check. - /// This test validates that depth-limit outside the valid range should fail validation - /// during `dab validate` and `dab start`. /// [DataTestMethod] - [DataRow(-1, true, DisplayName = "[PASS]: Valid Value: -1 to disable depth limit")] - [DataRow(0, false, DisplayName = "[FAIL]: Invalid Value: 0 for depth-limit.")] - [DataRow(-2, false, DisplayName = "[FAIL]: Invalid Value: -2 for depth-limit.")] - [DataRow(2, true, DisplayName = "[PASS]: Valid Value: 2 for depth-limit.")] - [DataRow(2147483647, true, DisplayName = "[PASS]: Valid Value: Using Int32.MaxValue(2147483647) for depth-limit.")] - [DataRow(null, true, DisplayName = "[PASS]: Default Value: null for depth-limit.")] + [DataRow(-1, DisplayName = "[PASS]: Valid Value: -1 to disable depth limit")] + [DataRow(2, DisplayName = "[PASS]: Valid Value: 2 for depth-limit.")] + [DataRow(2147483647, DisplayName = "[PASS]: Valid Value: Using Int32.MaxValue(2147483647) for depth-limit.")] + [DataRow(null, DisplayName = "[PASS]: Default Value: null for depth-limit.")] [TestCategory(TestCategory.MSSQL)] - public async Task TestValidateConfigForDifferentDepthLimit(int? depthLimit, bool isValidInput) + public async Task TestValidateConfigForValidDepthLimit(int? depthLimit) { - //Arrange - // Fetch the MS_SQL integration test config file. + await ValidateConfigWithDepthLimit(depthLimit, expectedSuccess: true); + } + + /// + /// This method validates that depth-limit outside the valid range should fail validation + /// during `dab validate` and `dab start`. + /// + /// + /// + private static async Task ValidateConfigWithDepthLimit(int? depthLimit, bool expectedSuccess) + { + // Arrange: Common setup logic TestHelper.SetupDatabaseEnvironment(MSSQL_ENVIRONMENT); const string CUSTOM_CONFIG = "custom-config.json"; FileSystemRuntimeConfigLoader testConfigPath = TestHelper.GetRuntimeConfigLoader(); @@ -1330,26 +1350,19 @@ public async Task TestValidateConfigForDifferentDepthLimit(int? depthLimit, bool }; MockFileSystem fileSystem = new(); - - // write the content of configuration to the custom-config file and add it to the filesystem. fileSystem.AddFile(CUSTOM_CONFIG, new MockFileData(configuration.ToJson())); FileSystemRuntimeConfigLoader configLoader = new(fileSystem); configLoader.UpdateConfigFilePath(CUSTOM_CONFIG); RuntimeConfigProvider configProvider = TestHelper.GetRuntimeConfigProvider(configLoader); Mock> configValidatorLogger = new(); - RuntimeConfigValidator configValidator = - new( - configProvider, - fileSystem, - configValidatorLogger.Object, - true); + RuntimeConfigValidator configValidator = new(configProvider, fileSystem, configValidatorLogger.Object, true); // Act bool isSuccess = await configValidator.TryValidateConfig(CUSTOM_CONFIG, TestHelper.ProvisionLoggerFactory()); - // Assert - Assert.AreEqual(isValidInput, isSuccess); + // Assert based on expected success + Assert.AreEqual(expectedSuccess, isSuccess); } /// @@ -3774,17 +3787,17 @@ public async Task ValidateNextLinkUsage() /// } /// /// The maximum allowed depth for GraphQL queries and mutations. - /// Indicates whether the operation is a mutation (true) or a query (false). + /// Indicates whether the operation is a mutation or a query. /// The expected HTTP status code for the operation. [DataTestMethod] - [DataRow(1, false, HttpStatusCode.BadRequest, DisplayName = "Failed Query execution when max depth limit is set to 1")] - [DataRow(2, false, HttpStatusCode.OK, DisplayName = "Query execution successful when max depth limit is set to 2")] - [DataRow(1, true, HttpStatusCode.BadRequest, DisplayName = "Failed Mutation execution when max depth limit is set to 1")] - [DataRow(2, true, HttpStatusCode.OK, DisplayName = "Mutation execution successful when max depth limit is set to 2")] + [DataRow(1, GraphQLOperation.Query, HttpStatusCode.BadRequest, DisplayName = "Failed Query execution when max depth limit is set to 1")] + [DataRow(2, GraphQLOperation.Query, HttpStatusCode.OK, DisplayName = "Query execution successful when max depth limit is set to 2")] + [DataRow(1, GraphQLOperation.Mutation, HttpStatusCode.BadRequest, DisplayName = "Failed Mutation execution when max depth limit is set to 1")] + [DataRow(2, GraphQLOperation.Mutation, HttpStatusCode.OK, DisplayName = "Mutation execution successful when max depth limit is set to 2")] [TestCategory(TestCategory.MSSQL)] public async Task TestDepthLimitRestrictionOnGraphQLInNonHostedMode( int depthLimit, - bool isMutationOperation, + GraphQLOperation operationType, HttpStatusCode expectedStatusCodeForGraphQL) { // Arrange @@ -3807,7 +3820,7 @@ public async Task TestDepthLimitRestrictionOnGraphQLInNonHostedMode( using (HttpClient client = server.CreateClient()) { string query; - if (isMutationOperation) + if (operationType is GraphQLOperation.Mutation) { // requested mutation operation has depth of 2 query = @"mutation createbook{