From ebe1ff050c4fbec1184d7e3565b41db5e2e8cb61 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Thu, 14 Jul 2022 15:58:29 -0700 Subject: [PATCH 01/24] Field AuthZ Mutations GraphQL --- .../Controllers/GraphQLController.cs | 7 ++- .../Resolvers/SqlMutationEngine.cs | 63 ++++++++++++++++++- .../Services/ResolverMiddleware.cs | 16 ++++- 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/DataGateway.Service/Controllers/GraphQLController.cs b/DataGateway.Service/Controllers/GraphQLController.cs index 5825d8de69..b01ac3b3b4 100644 --- a/DataGateway.Service/Controllers/GraphQLController.cs +++ b/DataGateway.Service/Controllers/GraphQLController.cs @@ -3,8 +3,10 @@ using System.Security.Claims; using System.Text.Json; using System.Threading.Tasks; +using Azure.DataGateway.Service.Authorization; using Azure.DataGateway.Service.Services; using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Primitives; namespace Azure.DataGateway.Service.Controllers { @@ -35,12 +37,15 @@ public async Task PostAsync() // recognizes the authenticated user. Anonymous requests are possible so check // for the HttpContext.User existence is necessary. Dictionary requestProperties = new(); + if (HttpContext.Request.Headers.TryGetValue(AuthorizationResolver.CLIENT_ROLE_HEADER, out StringValues clientRoleHeader)) + { + requestProperties.Add(key: "role", value: clientRoleHeader); + } if (this.HttpContext.User.Identity != null && this.HttpContext.User.Identity.IsAuthenticated) { requestProperties.Add(nameof(ClaimsPrincipal), this.HttpContext.User); } - // JsonElement returned so that JsonDocument is disposed when thread exits string resultJson = await this._schemaManager.ExecuteAsync(requestBody, requestProperties); using JsonDocument jsonDoc = JsonDocument.Parse(resultJson); diff --git a/DataGateway.Service/Resolvers/SqlMutationEngine.cs b/DataGateway.Service/Resolvers/SqlMutationEngine.cs index e412e470b1..4a04b936de 100644 --- a/DataGateway.Service/Resolvers/SqlMutationEngine.cs +++ b/DataGateway.Service/Resolvers/SqlMutationEngine.cs @@ -7,6 +7,7 @@ using System.Text; using System.Text.Json; using System.Threading.Tasks; +using Azure.DataGateway.Auth; using Azure.DataGateway.Config; using Azure.DataGateway.Service.Exceptions; using Azure.DataGateway.Service.GraphQLBuilder.Mutations; @@ -15,6 +16,7 @@ using HotChocolate.Resolvers; using HotChocolate.Types; using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Primitives; namespace Azure.DataGateway.Service.Resolvers { @@ -27,6 +29,10 @@ public class SqlMutationEngine : IMutationEngine private readonly ISqlMetadataProvider _sqlMetadataProvider; private readonly IQueryExecutor _queryExecutor; private readonly IQueryBuilder _queryBuilder; + private readonly IAuthorizationResolver _authorizationResolver; + + // Used to parse out GraphQL input query arguments. + public const string INPUT_ARGUMENT_NAME = "item"; /// /// Constructor @@ -35,12 +41,14 @@ public SqlMutationEngine( IQueryEngine queryEngine, IQueryExecutor queryExecutor, IQueryBuilder queryBuilder, - ISqlMetadataProvider sqlMetadataProvider) + ISqlMetadataProvider sqlMetadataProvider, + IAuthorizationResolver authorizationResolver) { _queryEngine = queryEngine; _queryExecutor = queryExecutor; _queryBuilder = queryBuilder; _sqlMetadataProvider = sqlMetadataProvider; + _authorizationResolver = authorizationResolver; } /// @@ -64,10 +72,61 @@ public async Task> ExecuteAsync(IMiddlewareContex MutationBuilder.DetermineMutationOperationTypeBasedOnInputType(graphqlMutationName); if (mutationOperation is Operation.Delete) { - // compute the mutation result before removing the element + // compute the mutation result before removing the element, + // since typical GraphQL delete mutations return the metadata of the deleted item. result = await _queryEngine.ExecuteAsync(context, parameters); } + string role = string.Empty; + if (context.ContextData.TryGetValue(key: "role", out object? value)) + { + if (value is not null) + { + role = (StringValues)value.ToString(); + } + } + + if (string.IsNullOrEmpty(role)) + { + throw new DataGatewayException( + message: "No ClientRoleHeader available to perform authorization.", + statusCode: HttpStatusCode.Unauthorized, + subStatusCode: DataGatewayException.SubStatusCodes.AuthorizationCheckFailed); + } + + List inputArgumentKeys = BaseSqlQueryStructure.InputArgumentToMutationParams(parameters, INPUT_ARGUMENT_NAME).Keys.ToList(); + bool isAuthorized = false; + switch (mutationOperation) + { + case Operation.UpdateGraphQL: + isAuthorized = _authorizationResolver.AreColumnsAllowedForAction(entityName, roleName: role, action: ActionType.UPDATE, inputArgumentKeys); + break; + case Operation.Create: + isAuthorized = _authorizationResolver.AreColumnsAllowedForAction(entityName, roleName: role, action: ActionType.CREATE, inputArgumentKeys); + break; + case Operation.Delete: + // Delete operations are not checked for authorization on field level, + // and instead at the mutation level and would be rejected before this time in the pipeline. + // Continuing on with operation. + isAuthorized = true; + break; + default: + throw new DataGatewayException( + message: "Invalid operation for GraphQL Mutation, must be Create, UpdateGraphQL, or Delete", + statusCode: HttpStatusCode.BadRequest, + subStatusCode: DataGatewayException.SubStatusCodes.BadRequest + ); + } + + if (!isAuthorized) + { + throw new DataGatewayException( + message: "Unauthorized due to one or more fields in this mutation.", + statusCode: HttpStatusCode.Unauthorized, + subStatusCode: DataGatewayException.SubStatusCodes.AuthorizationCheckFailed + ); + } + using DbDataReader dbDataReader = await PerformMutationOperation( entityName, diff --git a/DataGateway.Service/Services/ResolverMiddleware.cs b/DataGateway.Service/Services/ResolverMiddleware.cs index 3f5208ceda..d3231f2298 100644 --- a/DataGateway.Service/Services/ResolverMiddleware.cs +++ b/DataGateway.Service/Services/ResolverMiddleware.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Text.Json; using System.Threading.Tasks; +using Azure.DataGateway.Service.Authorization; using Azure.DataGateway.Service.GraphQLBuilder.CustomScalars; using Azure.DataGateway.Service.Models; using Azure.DataGateway.Service.Resolvers; @@ -9,6 +10,8 @@ using HotChocolate.Language; using HotChocolate.Resolvers; using HotChocolate.Types; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Primitives; namespace Azure.DataGateway.Service.Services { @@ -35,6 +38,15 @@ public ResolverMiddleware(FieldDelegate next, public async Task InvokeAsync(IMiddlewareContext context) { JsonElement jsonElement; + if (context.ContextData.TryGetValue("HttpContext", out object? value)) + { + if (value is not null) + { + HttpContext httpContext = (HttpContext)value; + StringValues clientRoleHeader = httpContext.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]; + context.ContextData.TryAdd(key: "role", value: clientRoleHeader); + } + } if (context.Selection.Field.Coordinate.TypeName.Value == "Mutation") { @@ -161,8 +173,10 @@ protected static bool IsInnerObject(IMiddlewareContext context) /// /// Extract parameters from the schema and the actual instance (query) of the field - /// Extracts defualt parameter values from the schema or null if no default + /// Extracts default parameter values from the schema or null if no default /// Overrides default values with actual values of parameters provided + /// Key: (string) argument field name + /// Value: (object) argument value /// public static IDictionary GetParametersFromSchemaAndQueryFields(IObjectField schema, FieldNode query, IVariableValueCollection variables) { From 84257b8e07c0d63490b2aeb31d48123fae738ef2 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 07:57:51 -0700 Subject: [PATCH 02/24] Updated test fixture setup with authorization resolver. --- DataGateway.Service.Tests/SqlTests/SqlTestBase.cs | 13 ++++++++----- .../Unittests/RestServiceUnitTests.cs | 6 ++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/DataGateway.Service.Tests/SqlTests/SqlTestBase.cs b/DataGateway.Service.Tests/SqlTests/SqlTestBase.cs index c5bf6f6d92..5f30c79e56 100644 --- a/DataGateway.Service.Tests/SqlTests/SqlTestBase.cs +++ b/DataGateway.Service.Tests/SqlTests/SqlTestBase.cs @@ -82,6 +82,12 @@ protected static async Task InitializeTestFixture(TestContext context, string te _httpContextAccessor = new Mock(); _httpContextAccessor.Setup(x => x.HttpContext.User).Returns(new ClaimsPrincipal()); + await ResetDbStateAsync(); + await _sqlMetadataProvider.InitializeAsync(); + + //Initialize the authorization resolver object + _authorizationResolver = new AuthorizationResolver(_runtimeConfigProvider, _sqlMetadataProvider); + _queryEngine = new SqlQueryEngine( _queryExecutor, _queryBuilder, @@ -92,12 +98,9 @@ protected static async Task InitializeTestFixture(TestContext context, string te _queryEngine, _queryExecutor, _queryBuilder, - _sqlMetadataProvider); - await ResetDbStateAsync(); - await _sqlMetadataProvider.InitializeAsync(); + _sqlMetadataProvider, + _authorizationResolver); - //Initialize the authorization resolver object - _authorizationResolver = new AuthorizationResolver(_runtimeConfigProvider, _sqlMetadataProvider); } protected static void SetUpSQLMetadataProvider() diff --git a/DataGateway.Service.Tests/Unittests/RestServiceUnitTests.cs b/DataGateway.Service.Tests/Unittests/RestServiceUnitTests.cs index 9f65cd80ed..e0d2e7f915 100644 --- a/DataGateway.Service.Tests/Unittests/RestServiceUnitTests.cs +++ b/DataGateway.Service.Tests/Unittests/RestServiceUnitTests.cs @@ -136,14 +136,16 @@ public static void InitializeTest(string path) sqlMetadataProvider, httpContextAccessor.Object); + AuthorizationResolver authZResolver = new(runtimeConfigProvider, sqlMetadataProvider); + SqlMutationEngine mutationEngine = new( queryEngine, queryExecutor, queryBuilder, - sqlMetadataProvider); + sqlMetadataProvider, + authZResolver); - AuthorizationResolver authZResolver = new(runtimeConfigProvider, sqlMetadataProvider); // Setup REST Service _restService = new RestService( queryEngine, From facf320b3ee5d077704852ba9f790a427fc9b148 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 08:11:35 -0700 Subject: [PATCH 03/24] rearrangement of test setup for PG tests. --- .../Configuration/ConfigurationTests.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs b/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs index bac4fea5eb..871cd21250 100644 --- a/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs +++ b/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs @@ -7,6 +7,7 @@ using System.Net.Http.Json; using System.Text.Json; using System.Threading.Tasks; +using Azure.DataGateway.Auth; using Azure.DataGateway.Config; using Azure.DataGateway.Service.Configurations; using Azure.DataGateway.Service.Controllers; @@ -142,9 +143,6 @@ public void TestLoadingLocalPostgresSettings() object queryEngine = server.Services.GetService(typeof(IQueryEngine)); Assert.IsInstanceOfType(queryEngine, typeof(SqlQueryEngine)); - object mutationEngine = server.Services.GetService(typeof(IMutationEngine)); - Assert.IsInstanceOfType(mutationEngine, typeof(SqlMutationEngine)); - object queryBuilder = server.Services.GetService(typeof(IQueryBuilder)); Assert.IsInstanceOfType(queryBuilder, typeof(PostgresQueryBuilder)); @@ -153,6 +151,12 @@ public void TestLoadingLocalPostgresSettings() object sqlMetadataProvider = server.Services.GetService(typeof(ISqlMetadataProvider)); Assert.IsInstanceOfType(sqlMetadataProvider, typeof(PostgreSqlMetadataProvider)); + + object authorizationResolver = server.Services.GetService(typeof(IAuthorizationResolver)); + Assert.IsInstanceOfType(authorizationResolver, typeof(IAuthorizationResolver)); + + object mutationEngine = server.Services.GetService(typeof(IMutationEngine)); + Assert.IsInstanceOfType(mutationEngine, typeof(SqlMutationEngine)); } [TestMethod("Validates that local MySql settings can be loaded and the correct classes are in the service provider.")] From c9dcb61dbc7a78c6e9ff2dbc14cb6a4d29eeaa70 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 09:32:18 -0700 Subject: [PATCH 04/24] Updating configuration overrides to adhere to validation. Policy referenced column that was explicilty disallowed. --- DataGateway.Service/hawaii-config.MySql.overrides.example.json | 3 +-- .../hawaii-config.PostgreSql.overrides.example.json | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/DataGateway.Service/hawaii-config.MySql.overrides.example.json b/DataGateway.Service/hawaii-config.MySql.overrides.example.json index be4d25dd4a..3cd2937a11 100644 --- a/DataGateway.Service/hawaii-config.MySql.overrides.example.json +++ b/DataGateway.Service/hawaii-config.MySql.overrides.example.json @@ -128,8 +128,7 @@ "database": "@claims.id eq @item.id" }, "fields": { - "include": [ "*" ], - "exclude": [ "id" ] + "include": [ "*" ] } } ] diff --git a/DataGateway.Service/hawaii-config.PostgreSql.overrides.example.json b/DataGateway.Service/hawaii-config.PostgreSql.overrides.example.json index 03226d9433..b2ed903057 100644 --- a/DataGateway.Service/hawaii-config.PostgreSql.overrides.example.json +++ b/DataGateway.Service/hawaii-config.PostgreSql.overrides.example.json @@ -128,8 +128,7 @@ "database": "@claims.id eq @item.id" }, "fields": { - "include": [ "*" ], - "exclude": [ "id" ] + "include": [ "*" ] } } ] From edbdb2982974c0e52f91c84af8b7ead53ad22ffe Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 09:55:13 -0700 Subject: [PATCH 05/24] Identifying GitHubActions PG failed configuration test cause, no repro'd locally. --- .../Configuration/ConfigurationTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs b/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs index 871cd21250..698dcb3634 100644 --- a/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs +++ b/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs @@ -140,6 +140,9 @@ public void TestLoadingLocalPostgresSettings() Environment.SetEnvironmentVariable(ASP_NET_CORE_ENVIRONMENT_VAR_NAME, POSTGRESQL_ENVIRONMENT); TestServer server = new(Program.CreateWebHostBuilder(Array.Empty())); + object sqlMetadataProvider = server.Services.GetService(typeof(ISqlMetadataProvider)); + Assert.IsInstanceOfType(sqlMetadataProvider, typeof(PostgreSqlMetadataProvider)); + object queryEngine = server.Services.GetService(typeof(IQueryEngine)); Assert.IsInstanceOfType(queryEngine, typeof(SqlQueryEngine)); @@ -149,9 +152,6 @@ public void TestLoadingLocalPostgresSettings() object queryExecutor = server.Services.GetService(typeof(IQueryExecutor)); Assert.IsInstanceOfType(queryExecutor, typeof(QueryExecutor)); - object sqlMetadataProvider = server.Services.GetService(typeof(ISqlMetadataProvider)); - Assert.IsInstanceOfType(sqlMetadataProvider, typeof(PostgreSqlMetadataProvider)); - object authorizationResolver = server.Services.GetService(typeof(IAuthorizationResolver)); Assert.IsInstanceOfType(authorizationResolver, typeof(IAuthorizationResolver)); From 4baae969857bcef97d0d03a53df7d258124061a0 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 10:10:07 -0700 Subject: [PATCH 06/24] remove authresolver from test to allow webhostbuilder to provide the instance. --- DataGateway.Service.Tests/Configuration/ConfigurationTests.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs b/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs index 698dcb3634..4d66f12c1c 100644 --- a/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs +++ b/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs @@ -152,9 +152,6 @@ public void TestLoadingLocalPostgresSettings() object queryExecutor = server.Services.GetService(typeof(IQueryExecutor)); Assert.IsInstanceOfType(queryExecutor, typeof(QueryExecutor)); - object authorizationResolver = server.Services.GetService(typeof(IAuthorizationResolver)); - Assert.IsInstanceOfType(authorizationResolver, typeof(IAuthorizationResolver)); - object mutationEngine = server.Services.GetService(typeof(IMutationEngine)); Assert.IsInstanceOfType(mutationEngine, typeof(SqlMutationEngine)); } From 2c42b93408f780e3f3ef381e68305b91635cfc62 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 10:13:14 -0700 Subject: [PATCH 07/24] fix using --- DataGateway.Service.Tests/Configuration/ConfigurationTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs b/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs index 4d66f12c1c..383c669d2d 100644 --- a/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs +++ b/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs @@ -7,7 +7,6 @@ using System.Net.Http.Json; using System.Text.Json; using System.Threading.Tasks; -using Azure.DataGateway.Auth; using Azure.DataGateway.Config; using Azure.DataGateway.Service.Configurations; using Azure.DataGateway.Service.Controllers; From f88d8c9a87fc3be64f645a36cab7b7ea9aade00f Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 10:29:58 -0700 Subject: [PATCH 08/24] Update ordering of service registration to accommodate setup of mutation engine instantiating singleton instance within ConfigureServices(), which has a dependency on IAuthorizationResolver. --- DataGateway.Service/Startup.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/DataGateway.Service/Startup.cs b/DataGateway.Service/Startup.cs index dd0152e7b6..d65ac7a078 100644 --- a/DataGateway.Service/Startup.cs +++ b/DataGateway.Service/Startup.cs @@ -75,6 +75,8 @@ public void ConfigureServices(IServiceCollection services) } }); + services.AddSingleton(); + services.AddSingleton(implementationFactory: (serviceProvider) => { RuntimeConfigProvider configProvider = serviceProvider.GetRequiredService(); @@ -185,7 +187,6 @@ public void ConfigureServices(IServiceCollection services) services.AddAuthorization(); services.AddSingleton(); - services.AddSingleton(); services.AddControllers(); } From e1dcb5c82f086bf7b566f216b1c3c3541221c6a2 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 11:01:02 -0700 Subject: [PATCH 09/24] reverting any changes to this test. --- .../Configuration/ConfigurationTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs b/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs index 383c669d2d..bac4fea5eb 100644 --- a/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs +++ b/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs @@ -139,20 +139,20 @@ public void TestLoadingLocalPostgresSettings() Environment.SetEnvironmentVariable(ASP_NET_CORE_ENVIRONMENT_VAR_NAME, POSTGRESQL_ENVIRONMENT); TestServer server = new(Program.CreateWebHostBuilder(Array.Empty())); - object sqlMetadataProvider = server.Services.GetService(typeof(ISqlMetadataProvider)); - Assert.IsInstanceOfType(sqlMetadataProvider, typeof(PostgreSqlMetadataProvider)); - object queryEngine = server.Services.GetService(typeof(IQueryEngine)); Assert.IsInstanceOfType(queryEngine, typeof(SqlQueryEngine)); + object mutationEngine = server.Services.GetService(typeof(IMutationEngine)); + Assert.IsInstanceOfType(mutationEngine, typeof(SqlMutationEngine)); + object queryBuilder = server.Services.GetService(typeof(IQueryBuilder)); Assert.IsInstanceOfType(queryBuilder, typeof(PostgresQueryBuilder)); object queryExecutor = server.Services.GetService(typeof(IQueryExecutor)); Assert.IsInstanceOfType(queryExecutor, typeof(QueryExecutor)); - object mutationEngine = server.Services.GetService(typeof(IMutationEngine)); - Assert.IsInstanceOfType(mutationEngine, typeof(SqlMutationEngine)); + object sqlMetadataProvider = server.Services.GetService(typeof(ISqlMetadataProvider)); + Assert.IsInstanceOfType(sqlMetadataProvider, typeof(PostgreSqlMetadataProvider)); } [TestMethod("Validates that local MySql settings can be loaded and the correct classes are in the service provider.")] From d951a6480528a4d00a124ceb8eec408b833a2695 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 11:18:20 -0700 Subject: [PATCH 10/24] logging. --- DataGateway.Service/Program.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/DataGateway.Service/Program.cs b/DataGateway.Service/Program.cs index 134c33c14f..b8ff2ebe47 100644 --- a/DataGateway.Service/Program.cs +++ b/DataGateway.Service/Program.cs @@ -76,6 +76,7 @@ private static void AddConfigurationProviders( { string configFileName = RuntimeConfigPath.GetFileNameForEnvironment(env.EnvironmentName); + Console.WriteLine("CONFIGFILE: " + configFileName); Dictionary configFileNameMap = new() { { From c13c62393dd6131e75aeb9fb25ae2422f7f58056 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 11:32:48 -0700 Subject: [PATCH 11/24] Adding test categories to config tests that now depend on authorization resolver for mutationengine. Connection strings are improperly resolved in unit tests run via Build GitHub action. These 3 tests should now run with the integration tests which properly resolve a correctly formatted connection string. --- .../Configuration/ConfigurationTests.cs | 6 +++--- DataGateway.Service/Program.cs | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs b/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs index bac4fea5eb..2b615ca0cf 100644 --- a/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs +++ b/DataGateway.Service.Tests/Configuration/ConfigurationTests.cs @@ -111,7 +111,7 @@ public void TestLoadingLocalCosmosSettings() ValidateCosmosDbSetup(server); } - [TestMethod("Validates that local MsSql settings can be loaded and the correct classes are in the service provider.")] + [TestMethod("Validates that local MsSql settings can be loaded and the correct classes are in the service provider."), TestCategory(TestCategory.MSSQL)] public void TestLoadingLocalMsSqlSettings() { Environment.SetEnvironmentVariable(ASP_NET_CORE_ENVIRONMENT_VAR_NAME, MSSQL_ENVIRONMENT); @@ -133,7 +133,7 @@ public void TestLoadingLocalMsSqlSettings() Assert.IsInstanceOfType(sqlMetadataProvider, typeof(MsSqlMetadataProvider)); } - [TestMethod("Validates that local PostgreSql settings can be loaded and the correct classes are in the service provider.")] + [TestMethod("Validates that local PostgreSql settings can be loaded and the correct classes are in the service provider."), TestCategory(TestCategory.POSTGRESQL)] public void TestLoadingLocalPostgresSettings() { Environment.SetEnvironmentVariable(ASP_NET_CORE_ENVIRONMENT_VAR_NAME, POSTGRESQL_ENVIRONMENT); @@ -155,7 +155,7 @@ public void TestLoadingLocalPostgresSettings() Assert.IsInstanceOfType(sqlMetadataProvider, typeof(PostgreSqlMetadataProvider)); } - [TestMethod("Validates that local MySql settings can be loaded and the correct classes are in the service provider.")] + [TestMethod("Validates that local MySql settings can be loaded and the correct classes are in the service provider."), TestCategory(TestCategory.MYSQL)] public void TestLoadingLocalMySqlSettings() { Environment.SetEnvironmentVariable(ASP_NET_CORE_ENVIRONMENT_VAR_NAME, MYSQL_ENVIRONMENT); diff --git a/DataGateway.Service/Program.cs b/DataGateway.Service/Program.cs index b8ff2ebe47..134c33c14f 100644 --- a/DataGateway.Service/Program.cs +++ b/DataGateway.Service/Program.cs @@ -76,7 +76,6 @@ private static void AddConfigurationProviders( { string configFileName = RuntimeConfigPath.GetFileNameForEnvironment(env.EnvironmentName); - Console.WriteLine("CONFIGFILE: " + configFileName); Dictionary configFileNameMap = new() { { From 35746897438918124319c264eb58697b8155f6de Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 13:59:54 -0700 Subject: [PATCH 12/24] Adding Unit Tests. --- .../GraphQLMutationAuthorizationTests.cs | 104 +++++++++++++++ .../Resolvers/SqlMutationEngine.cs | 119 ++++++++++-------- 2 files changed, 174 insertions(+), 49 deletions(-) create mode 100644 DataGateway.Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationAuthorizationTests.cs diff --git a/DataGateway.Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationAuthorizationTests.cs b/DataGateway.Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationAuthorizationTests.cs new file mode 100644 index 0000000000..a91dc7b9fa --- /dev/null +++ b/DataGateway.Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationAuthorizationTests.cs @@ -0,0 +1,104 @@ +#nullable enable +using System.Collections.Generic; +using Azure.DataGateway.Auth; +using Azure.DataGateway.Service.Exceptions; +using Azure.DataGateway.Service.Resolvers; +using Azure.DataGateway.Service.Services; +using HotChocolate.Language; +using HotChocolate.Resolvers; +using Microsoft.Extensions.Primitives; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Azure.DataGateway.Service.Tests.SqlTests.GraphQLMutationTests +{ + /// + /// Base class for GraphQL Mutation tests targetting Sql databases. + /// + [TestClass] + public class GraphQLMutationAuthorizationTests : SqlTestBase + { + private const string TEST_ENTITY = "TEST_ENTITY"; + private const string TEST_COLUMN_VALUE = "COLUMN_VALUE"; + private const string MIDDLEWARE_CONTEXT_ROLEHEADER_KEY = "role"; + private const string MIDDLEWARE_CONTEXT_ROLEHEADER_VALUE = "roleName"; + + [DataTestMethod] + [DataRow(Config.Operation.Create, true, new string[] { "col1", "col2", "col3" }, new string[] { "col1" }, DisplayName = "Anonymous Mutation Fields")] + [DataRow(false, new string[] { "col1", "col2", "col3" }, new string[] { "col4" }, DisplayName = "Anonymous Mutation Fields")] + + public void MutationFields_AuthorizationEvaluation(bool isAuthorized, string[] columnsAllowed, string[] columnsRequested) + { + SqlMutationEngine engine = SetupTestFixture(isAuthorized); + + // Setup mock mutation input, utilized in BaseSqlQueryStructure.InputArgumentToMutationParams() helper. + List mutationInputRaw = new(); + foreach (string column in columnsRequested) + { + mutationInputRaw.Add(new ObjectFieldNode(name: column, value: TEST_COLUMN_VALUE)); + } + + Dictionary parameters = new() + { + { SqlMutationEngine.INPUT_ARGUMENT_NAME, mutationInputRaw } + }; + + Dictionary middlewareContextData = new() + { + { MIDDLEWARE_CONTEXT_ROLEHEADER_KEY, new StringValues(MIDDLEWARE_CONTEXT_ROLEHEADER_VALUE) } + }; + + Mock graphQLMiddlewareContext = new(); + graphQLMiddlewareContext.Setup(x => x.ContextData).Returns(middlewareContextData); + + bool authorizationResult = false; + try + { + engine.AuthorizeMutationFields( + graphQLMiddlewareContext.Object, + parameters, + entityName: TEST_ENTITY, + mutationOperation: Config.Operation.Create + ); + + authorizationResult = true; + } + catch (DataGatewayException) + { + Assert.IsFalse(isAuthorized, message: "Mutation fields authorized erroneously."); + } + + Assert.AreEqual(actual: authorizationResult, expected: isAuthorized, message: "Mutation field authorization incorrectly evaluated."); + } + + /// + /// Sets up test fixture for class, only to be run once per test run, as defined by + /// MSTest decorator. + /// + /// + private SqlMutationEngine SetupTestFixture(bool areColumnsAllowed) + { + Mock _queryEngine = new(); + Mock _sqlMetadataProvider = new(); + Mock _queryExecutor = new(); + Mock _queryBuilder = new(); + + // Creates Mock AuthorizationResolver to return a preset result based on [TestMethod] input. + Mock _authorizationResolver = new(); + _authorizationResolver.Setup(x => x.AreColumnsAllowedForAction( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>() // Can be any IEnumerable, as find request result field list is depedent on AllowedColumns. + )).Returns(areColumnsAllowed); + + return new SqlMutationEngine( + _queryEngine.Object, + _queryExecutor.Object, + _queryBuilder.Object, + _sqlMetadataProvider.Object, + _authorizationResolver.Object + ); + } + } +} diff --git a/DataGateway.Service/Resolvers/SqlMutationEngine.cs b/DataGateway.Service/Resolvers/SqlMutationEngine.cs index 4a04b936de..5cc05aca96 100644 --- a/DataGateway.Service/Resolvers/SqlMutationEngine.cs +++ b/DataGateway.Service/Resolvers/SqlMutationEngine.cs @@ -77,55 +77,8 @@ public async Task> ExecuteAsync(IMiddlewareContex result = await _queryEngine.ExecuteAsync(context, parameters); } - string role = string.Empty; - if (context.ContextData.TryGetValue(key: "role", out object? value)) - { - if (value is not null) - { - role = (StringValues)value.ToString(); - } - } - - if (string.IsNullOrEmpty(role)) - { - throw new DataGatewayException( - message: "No ClientRoleHeader available to perform authorization.", - statusCode: HttpStatusCode.Unauthorized, - subStatusCode: DataGatewayException.SubStatusCodes.AuthorizationCheckFailed); - } - - List inputArgumentKeys = BaseSqlQueryStructure.InputArgumentToMutationParams(parameters, INPUT_ARGUMENT_NAME).Keys.ToList(); - bool isAuthorized = false; - switch (mutationOperation) - { - case Operation.UpdateGraphQL: - isAuthorized = _authorizationResolver.AreColumnsAllowedForAction(entityName, roleName: role, action: ActionType.UPDATE, inputArgumentKeys); - break; - case Operation.Create: - isAuthorized = _authorizationResolver.AreColumnsAllowedForAction(entityName, roleName: role, action: ActionType.CREATE, inputArgumentKeys); - break; - case Operation.Delete: - // Delete operations are not checked for authorization on field level, - // and instead at the mutation level and would be rejected before this time in the pipeline. - // Continuing on with operation. - isAuthorized = true; - break; - default: - throw new DataGatewayException( - message: "Invalid operation for GraphQL Mutation, must be Create, UpdateGraphQL, or Delete", - statusCode: HttpStatusCode.BadRequest, - subStatusCode: DataGatewayException.SubStatusCodes.BadRequest - ); - } - - if (!isAuthorized) - { - throw new DataGatewayException( - message: "Unauthorized due to one or more fields in this mutation.", - statusCode: HttpStatusCode.Unauthorized, - subStatusCode: DataGatewayException.SubStatusCodes.AuthorizationCheckFailed - ); - } + // If authorization fails, an exception will be thrown and request execution halts. + AuthorizeMutationFields(context, parameters, entityName, mutationOperation); using DbDataReader dbDataReader = await PerformMutationOperation( @@ -435,5 +388,73 @@ public string ConstructPrimaryKeyRoute(string entityName, Dictionary + /// Authorization check on mutation fields provided in a GraphQL Mutation request. + /// + /// + /// + /// + /// + /// + /// + /// + public void AuthorizeMutationFields( + IMiddlewareContext context, + IDictionary parameters, + string entityName, + Operation mutationOperation) + { + string role = string.Empty; + if (context.ContextData.TryGetValue(key: "role", out object? value)) + { + if (value is not null) + { + role = (StringValues)value.ToString(); + } + } + + if (string.IsNullOrEmpty(role)) + { + throw new DataGatewayException( + message: "No ClientRoleHeader available to perform authorization.", + statusCode: HttpStatusCode.Unauthorized, + subStatusCode: DataGatewayException.SubStatusCodes.AuthorizationCheckFailed); + } + + List inputArgumentKeys = BaseSqlQueryStructure.InputArgumentToMutationParams(parameters, INPUT_ARGUMENT_NAME).Keys.ToList(); + bool isAuthorized; + + switch (mutationOperation) + { + case Operation.UpdateGraphQL: + isAuthorized = _authorizationResolver.AreColumnsAllowedForAction(entityName, roleName: role, action: ActionType.UPDATE, inputArgumentKeys); + break; + case Operation.Create: + isAuthorized = _authorizationResolver.AreColumnsAllowedForAction(entityName, roleName: role, action: ActionType.CREATE, inputArgumentKeys); + break; + case Operation.Delete: + // Delete operations are not checked for authorization on field level, + // and instead at the mutation level and would be rejected before this time in the pipeline. + // Continuing on with operation. + isAuthorized = true; + break; + default: + throw new DataGatewayException( + message: "Invalid operation for GraphQL Mutation, must be Create, UpdateGraphQL, or Delete", + statusCode: HttpStatusCode.BadRequest, + subStatusCode: DataGatewayException.SubStatusCodes.BadRequest + ); + } + + if (!isAuthorized) + { + throw new DataGatewayException( + message: "Unauthorized due to one or more fields in this mutation.", + statusCode: HttpStatusCode.Forbidden, + subStatusCode: DataGatewayException.SubStatusCodes.AuthorizationCheckFailed + ); + } + } } } From b6e5c4c25c26a4e5d1d17b1f949890847dec8b06 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 14:06:24 -0700 Subject: [PATCH 13/24] moving test location and adjust params. --- .../GraphQL}/GraphQLMutationAuthorizationTests.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) rename DataGateway.Service.Tests/{SqlTests/GraphQLMutationTests => Authorization/GraphQL}/GraphQLMutationAuthorizationTests.cs (89%) diff --git a/DataGateway.Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationAuthorizationTests.cs b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs similarity index 89% rename from DataGateway.Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationAuthorizationTests.cs rename to DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs index a91dc7b9fa..5d9fed5a9e 100644 --- a/DataGateway.Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationAuthorizationTests.cs +++ b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs @@ -4,13 +4,14 @@ using Azure.DataGateway.Service.Exceptions; using Azure.DataGateway.Service.Resolvers; using Azure.DataGateway.Service.Services; +using Azure.DataGateway.Service.Tests.SqlTests; using HotChocolate.Language; using HotChocolate.Resolvers; using Microsoft.Extensions.Primitives; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -namespace Azure.DataGateway.Service.Tests.SqlTests.GraphQLMutationTests +namespace Azure.DataGateway.Service.Tests.Authorization.GraphQL { /// /// Base class for GraphQL Mutation tests targetting Sql databases. @@ -24,10 +25,10 @@ public class GraphQLMutationAuthorizationTests : SqlTestBase private const string MIDDLEWARE_CONTEXT_ROLEHEADER_VALUE = "roleName"; [DataTestMethod] - [DataRow(Config.Operation.Create, true, new string[] { "col1", "col2", "col3" }, new string[] { "col1" }, DisplayName = "Anonymous Mutation Fields")] - [DataRow(false, new string[] { "col1", "col2", "col3" }, new string[] { "col4" }, DisplayName = "Anonymous Mutation Fields")] + [DataRow(true, new string[] { "col1", "col2", "col3" }, new string[] { "col1" }, Config.Operation.Create, DisplayName = "Create Mutation Field Authorization")] + [DataRow(false, new string[] { "col1", "col2", "col3" }, new string[] { "col4" }, Config.Operation.Update, DisplayName = "Update Mutation Field Authorization")] - public void MutationFields_AuthorizationEvaluation(bool isAuthorized, string[] columnsAllowed, string[] columnsRequested) + public void MutationFields_AuthorizationEvaluation(bool isAuthorized, string[] columnsAllowed, string[] columnsRequested, Config.Operation operation) { SqlMutationEngine engine = SetupTestFixture(isAuthorized); @@ -58,7 +59,7 @@ public void MutationFields_AuthorizationEvaluation(bool isAuthorized, string[] c graphQLMiddlewareContext.Object, parameters, entityName: TEST_ENTITY, - mutationOperation: Config.Operation.Create + mutationOperation: operation ); authorizationResult = true; From 9e5ccad968e1680d617f6152a32fd69137506cc2 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 14:48:24 -0700 Subject: [PATCH 14/24] UPdate test. --- .../GraphQLMutationAuthorizationTests.cs | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs index 5d9fed5a9e..0b372f2f17 100644 --- a/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs +++ b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs @@ -1,4 +1,5 @@ #nullable enable +using System; using System.Collections.Generic; using Azure.DataGateway.Auth; using Azure.DataGateway.Service.Exceptions; @@ -14,25 +15,40 @@ namespace Azure.DataGateway.Service.Tests.Authorization.GraphQL { /// - /// Base class for GraphQL Mutation tests targetting Sql databases. + /// Unit Tests validating mutation field authorization for GraphQL. + /// Ensures the authorization decision from the authorizationResolver properly triggers + /// an exception for failure (DataGatewayException.Forbidden), and proceeds normally for success. /// [TestClass] - public class GraphQLMutationAuthorizationTests : SqlTestBase + public class GraphQLMutationAuthorizationTests { private const string TEST_ENTITY = "TEST_ENTITY"; private const string TEST_COLUMN_VALUE = "COLUMN_VALUE"; private const string MIDDLEWARE_CONTEXT_ROLEHEADER_KEY = "role"; private const string MIDDLEWARE_CONTEXT_ROLEHEADER_VALUE = "roleName"; + /// + /// This test ensures that data passed into AuthorizeMutationFields() within the SqlMutationEngine + /// are evaluated and provided to the authorization resolver for an authorization decision. + /// If authorization fails, an exception is thrown and this test validates that scenario. + /// If authorization succeeds, no exceptions are thrown for authorization, and function resolves silently. + /// + /// + /// + /// + /// [DataTestMethod] - [DataRow(true, new string[] { "col1", "col2", "col3" }, new string[] { "col1" }, Config.Operation.Create, DisplayName = "Create Mutation Field Authorization")] - [DataRow(false, new string[] { "col1", "col2", "col3" }, new string[] { "col4" }, Config.Operation.Update, DisplayName = "Update Mutation Field Authorization")] + [DataRow(true, new string[] { "col1", "col2", "col3" }, new string[] { "col1" }, Config.Operation.Create, DisplayName = "Create Mutation Field Authorization - Success, Columns Allowed")] + [DataRow(false, new string[] { "col1", "col2", "col3" }, new string[] { "col1" }, Config.Operation.Create, DisplayName = "Create Mutation Field Authorization - Failure, Columns Forbidden")] + [DataRow(true, new string[] { "col1", "col2", "col3" }, new string[] { "col1" }, Config.Operation.UpdateGraphQL, DisplayName = "Update Mutation Field Authorization - Success, Columns Allowed")] + [DataRow(false, new string[] { "col1", "col2", "col3" }, new string[] { "col4" }, Config.Operation.UpdateGraphQL, DisplayName = "Update Mutation Field Authorization - Failure, Columns Forbidden")] public void MutationFields_AuthorizationEvaluation(bool isAuthorized, string[] columnsAllowed, string[] columnsRequested, Config.Operation operation) { SqlMutationEngine engine = SetupTestFixture(isAuthorized); // Setup mock mutation input, utilized in BaseSqlQueryStructure.InputArgumentToMutationParams() helper. + // This takes the test's "columnsRequested" and adds them to the mutation input. List mutationInputRaw = new(); foreach (string column in columnsRequested) { @@ -64,9 +80,10 @@ public void MutationFields_AuthorizationEvaluation(bool isAuthorized, string[] c authorizationResult = true; } - catch (DataGatewayException) + catch (DataGatewayException dgException) { - Assert.IsFalse(isAuthorized, message: "Mutation fields authorized erroneously."); + Console.Error.WriteLine(dgException.Message); + Assert.IsFalse(isAuthorized, message: "Mutation fields authorized erroneously, no exception expected."); } Assert.AreEqual(actual: authorizationResult, expected: isAuthorized, message: "Mutation field authorization incorrectly evaluated."); @@ -77,7 +94,7 @@ public void MutationFields_AuthorizationEvaluation(bool isAuthorized, string[] c /// MSTest decorator. /// /// - private SqlMutationEngine SetupTestFixture(bool areColumnsAllowed) + private static SqlMutationEngine SetupTestFixture(bool isAuthorized) { Mock _queryEngine = new(); Mock _sqlMetadataProvider = new(); @@ -91,7 +108,7 @@ private SqlMutationEngine SetupTestFixture(bool areColumnsAllowed) It.IsAny(), It.IsAny(), It.IsAny>() // Can be any IEnumerable, as find request result field list is depedent on AllowedColumns. - )).Returns(areColumnsAllowed); + )).Returns(isAuthorized); return new SqlMutationEngine( _queryEngine.Object, From 1589f77db28126667c18a14238c631ecf6ab48f5 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Fri, 15 Jul 2022 14:54:43 -0700 Subject: [PATCH 15/24] fix using --- .../Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs index 0b372f2f17..1e950710c6 100644 --- a/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs +++ b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs @@ -5,7 +5,6 @@ using Azure.DataGateway.Service.Exceptions; using Azure.DataGateway.Service.Resolvers; using Azure.DataGateway.Service.Services; -using Azure.DataGateway.Service.Tests.SqlTests; using HotChocolate.Language; using HotChocolate.Resolvers; using Microsoft.Extensions.Primitives; From f010ab256d279b26d80a87493912f53efe5ce02e Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Mon, 18 Jul 2022 08:09:37 -0700 Subject: [PATCH 16/24] Update const string for "item" input argument name. Least impactful change rather than pulling out constants from Update/CreateMutaitonBuilder classes and tests. --- .../Mutations/MutationBuilder.cs | 2 ++ .../GraphQL/GraphQLMutationAuthorizationTests.cs | 3 ++- DataGateway.Service/Resolvers/SqlMutationEngine.cs | 5 +---- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/DataGateway.Service.GraphQLBuilder/Mutations/MutationBuilder.cs b/DataGateway.Service.GraphQLBuilder/Mutations/MutationBuilder.cs index d28c7680c4..964aa269c8 100644 --- a/DataGateway.Service.GraphQLBuilder/Mutations/MutationBuilder.cs +++ b/DataGateway.Service.GraphQLBuilder/Mutations/MutationBuilder.cs @@ -8,6 +8,8 @@ namespace Azure.DataGateway.Service.GraphQLBuilder.Mutations { public static class MutationBuilder { + public const string INPUT_ARGUMENT_NAME = "item"; + /// /// Creates a DocumentNode containing FieldDefinitionNodes representing mutations /// diff --git a/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs index 1e950710c6..bd03bc4ef8 100644 --- a/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs +++ b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using Azure.DataGateway.Auth; using Azure.DataGateway.Service.Exceptions; +using Azure.DataGateway.Service.GraphQLBuilder.Mutations; using Azure.DataGateway.Service.Resolvers; using Azure.DataGateway.Service.Services; using HotChocolate.Language; @@ -56,7 +57,7 @@ public void MutationFields_AuthorizationEvaluation(bool isAuthorized, string[] c Dictionary parameters = new() { - { SqlMutationEngine.INPUT_ARGUMENT_NAME, mutationInputRaw } + { MutationBuilder.INPUT_ARGUMENT_NAME, mutationInputRaw } }; Dictionary middlewareContextData = new() diff --git a/DataGateway.Service/Resolvers/SqlMutationEngine.cs b/DataGateway.Service/Resolvers/SqlMutationEngine.cs index 5cc05aca96..4a7dc33976 100644 --- a/DataGateway.Service/Resolvers/SqlMutationEngine.cs +++ b/DataGateway.Service/Resolvers/SqlMutationEngine.cs @@ -31,9 +31,6 @@ public class SqlMutationEngine : IMutationEngine private readonly IQueryBuilder _queryBuilder; private readonly IAuthorizationResolver _authorizationResolver; - // Used to parse out GraphQL input query arguments. - public const string INPUT_ARGUMENT_NAME = "item"; - /// /// Constructor /// @@ -422,7 +419,7 @@ public void AuthorizeMutationFields( subStatusCode: DataGatewayException.SubStatusCodes.AuthorizationCheckFailed); } - List inputArgumentKeys = BaseSqlQueryStructure.InputArgumentToMutationParams(parameters, INPUT_ARGUMENT_NAME).Keys.ToList(); + List inputArgumentKeys = BaseSqlQueryStructure.InputArgumentToMutationParams(parameters, MutationBuilder.INPUT_ARGUMENT_NAME).Keys.ToList(); bool isAuthorized; switch (mutationOperation) From dd90489b70f498979dcd7299b68870add9fbb784 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Mon, 18 Jul 2022 08:14:53 -0700 Subject: [PATCH 17/24] comment on default value false of bool. --- DataGateway.Service/Resolvers/SqlMutationEngine.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DataGateway.Service/Resolvers/SqlMutationEngine.cs b/DataGateway.Service/Resolvers/SqlMutationEngine.cs index 4a7dc33976..e56e0042b1 100644 --- a/DataGateway.Service/Resolvers/SqlMutationEngine.cs +++ b/DataGateway.Service/Resolvers/SqlMutationEngine.cs @@ -420,7 +420,7 @@ public void AuthorizeMutationFields( } List inputArgumentKeys = BaseSqlQueryStructure.InputArgumentToMutationParams(parameters, MutationBuilder.INPUT_ARGUMENT_NAME).Keys.ToList(); - bool isAuthorized; + bool isAuthorized; // False by default. switch (mutationOperation) { From 80bd23579d49890c47be76065986b8b653984941 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Mon, 18 Jul 2022 08:15:09 -0700 Subject: [PATCH 18/24] Remove erroneous 'find' comment on test mock. --- .../Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs index bd03bc4ef8..b3f1aaf529 100644 --- a/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs +++ b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs @@ -107,7 +107,7 @@ private static SqlMutationEngine SetupTestFixture(bool isAuthorized) It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny>() // Can be any IEnumerable, as find request result field list is depedent on AllowedColumns. + It.IsAny>() )).Returns(isAuthorized); return new SqlMutationEngine( From 826c74bbe27716091abbf0b6bd33cf32ac701db1 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Mon, 18 Jul 2022 08:18:11 -0700 Subject: [PATCH 19/24] use `AuthorizationResolver.CLIENT_ROLE_HEADER` const --- .../GraphQL/GraphQLMutationAuthorizationTests.cs | 4 ++-- DataGateway.Service/Resolvers/SqlMutationEngine.cs | 3 ++- DataGateway.Service/Services/ResolverMiddleware.cs | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs index b3f1aaf529..ad88616876 100644 --- a/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs +++ b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using Azure.DataGateway.Auth; +using Azure.DataGateway.Service.Authorization; using Azure.DataGateway.Service.Exceptions; using Azure.DataGateway.Service.GraphQLBuilder.Mutations; using Azure.DataGateway.Service.Resolvers; @@ -24,7 +25,6 @@ public class GraphQLMutationAuthorizationTests { private const string TEST_ENTITY = "TEST_ENTITY"; private const string TEST_COLUMN_VALUE = "COLUMN_VALUE"; - private const string MIDDLEWARE_CONTEXT_ROLEHEADER_KEY = "role"; private const string MIDDLEWARE_CONTEXT_ROLEHEADER_VALUE = "roleName"; /// @@ -62,7 +62,7 @@ public void MutationFields_AuthorizationEvaluation(bool isAuthorized, string[] c Dictionary middlewareContextData = new() { - { MIDDLEWARE_CONTEXT_ROLEHEADER_KEY, new StringValues(MIDDLEWARE_CONTEXT_ROLEHEADER_VALUE) } + { AuthorizationResolver.CLIENT_ROLE_HEADER, new StringValues(MIDDLEWARE_CONTEXT_ROLEHEADER_VALUE) } }; Mock graphQLMiddlewareContext = new(); diff --git a/DataGateway.Service/Resolvers/SqlMutationEngine.cs b/DataGateway.Service/Resolvers/SqlMutationEngine.cs index e56e0042b1..9fa1b19acc 100644 --- a/DataGateway.Service/Resolvers/SqlMutationEngine.cs +++ b/DataGateway.Service/Resolvers/SqlMutationEngine.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using Azure.DataGateway.Auth; using Azure.DataGateway.Config; +using Azure.DataGateway.Service.Authorization; using Azure.DataGateway.Service.Exceptions; using Azure.DataGateway.Service.GraphQLBuilder.Mutations; using Azure.DataGateway.Service.Models; @@ -403,7 +404,7 @@ public void AuthorizeMutationFields( Operation mutationOperation) { string role = string.Empty; - if (context.ContextData.TryGetValue(key: "role", out object? value)) + if (context.ContextData.TryGetValue(key: AuthorizationResolver.CLIENT_ROLE_HEADER, out object? value)) { if (value is not null) { diff --git a/DataGateway.Service/Services/ResolverMiddleware.cs b/DataGateway.Service/Services/ResolverMiddleware.cs index d3231f2298..00a3515ccc 100644 --- a/DataGateway.Service/Services/ResolverMiddleware.cs +++ b/DataGateway.Service/Services/ResolverMiddleware.cs @@ -44,7 +44,7 @@ public async Task InvokeAsync(IMiddlewareContext context) { HttpContext httpContext = (HttpContext)value; StringValues clientRoleHeader = httpContext.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]; - context.ContextData.TryAdd(key: "role", value: clientRoleHeader); + context.ContextData.TryAdd(key: AuthorizationResolver.CLIENT_ROLE_HEADER, value: clientRoleHeader); } } From 5de9817f587a4c24ff08507e0b688f3d1c015e77 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Mon, 18 Jul 2022 09:37:44 -0700 Subject: [PATCH 20/24] Fix straggling magic string: "role" and added constant for client role header. --- DataGateway.Service/Controllers/GraphQLController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DataGateway.Service/Controllers/GraphQLController.cs b/DataGateway.Service/Controllers/GraphQLController.cs index b01ac3b3b4..72fa16fea8 100644 --- a/DataGateway.Service/Controllers/GraphQLController.cs +++ b/DataGateway.Service/Controllers/GraphQLController.cs @@ -39,7 +39,7 @@ public async Task PostAsync() Dictionary requestProperties = new(); if (HttpContext.Request.Headers.TryGetValue(AuthorizationResolver.CLIENT_ROLE_HEADER, out StringValues clientRoleHeader)) { - requestProperties.Add(key: "role", value: clientRoleHeader); + requestProperties.Add(key: AuthorizationResolver.CLIENT_ROLE_HEADER, value: clientRoleHeader); } if (this.HttpContext.User.Identity != null && this.HttpContext.User.Identity.IsAuthenticated) From 2880dff3d9f2ef16682ee90250346670fd31468e Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Mon, 18 Jul 2022 09:48:57 -0700 Subject: [PATCH 21/24] Add delete codepath test. --- .../Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs index ad88616876..9d083a5341 100644 --- a/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs +++ b/DataGateway.Service.Tests/Authorization/GraphQL/GraphQLMutationAuthorizationTests.cs @@ -42,7 +42,8 @@ public class GraphQLMutationAuthorizationTests [DataRow(false, new string[] { "col1", "col2", "col3" }, new string[] { "col1" }, Config.Operation.Create, DisplayName = "Create Mutation Field Authorization - Failure, Columns Forbidden")] [DataRow(true, new string[] { "col1", "col2", "col3" }, new string[] { "col1" }, Config.Operation.UpdateGraphQL, DisplayName = "Update Mutation Field Authorization - Success, Columns Allowed")] [DataRow(false, new string[] { "col1", "col2", "col3" }, new string[] { "col4" }, Config.Operation.UpdateGraphQL, DisplayName = "Update Mutation Field Authorization - Failure, Columns Forbidden")] - + [DataRow(true, new string[] { "col1", "col2", "col3" }, new string[] { "col1" }, Config.Operation.Delete, DisplayName = "Delete Mutation Field Authorization - Success, since authorization to perform the" + + "delete mutation operation occurs prior to column evaluation in the request pipeline.")] public void MutationFields_AuthorizationEvaluation(bool isAuthorized, string[] columnsAllowed, string[] columnsRequested, Config.Operation operation) { SqlMutationEngine engine = SetupTestFixture(isAuthorized); From 20fd12bdcc5414ff2f2896bf52c1347e6bec58ad Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Mon, 18 Jul 2022 12:16:32 -0700 Subject: [PATCH 22/24] Add null forgiving operator instead of IF for CS8602 warning of possible dereference of null value. --- DataGateway.Service/Resolvers/SqlMutationEngine.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/DataGateway.Service/Resolvers/SqlMutationEngine.cs b/DataGateway.Service/Resolvers/SqlMutationEngine.cs index 9fa1b19acc..9d1dad713d 100644 --- a/DataGateway.Service/Resolvers/SqlMutationEngine.cs +++ b/DataGateway.Service/Resolvers/SqlMutationEngine.cs @@ -406,10 +406,7 @@ public void AuthorizeMutationFields( string role = string.Empty; if (context.ContextData.TryGetValue(key: AuthorizationResolver.CLIENT_ROLE_HEADER, out object? value)) { - if (value is not null) - { - role = (StringValues)value.ToString(); - } + role = (StringValues)value!.ToString(); } if (string.IsNullOrEmpty(role)) From 637cd71957a678eb717faec2fa3623e033c8494f Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Mon, 18 Jul 2022 13:29:53 -0700 Subject: [PATCH 23/24] Revert moving IAuthorizationResolver Add singleton as it actually does not have an effect on creation logic for mutationEngine --- DataGateway.Service/Startup.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/DataGateway.Service/Startup.cs b/DataGateway.Service/Startup.cs index c9c88bc743..cddff7eca8 100644 --- a/DataGateway.Service/Startup.cs +++ b/DataGateway.Service/Startup.cs @@ -75,8 +75,6 @@ public void ConfigureServices(IServiceCollection services) } }); - services.AddSingleton(); - services.AddSingleton(implementationFactory: (serviceProvider) => { RuntimeConfigProvider configProvider = serviceProvider.GetRequiredService(); @@ -187,6 +185,7 @@ public void ConfigureServices(IServiceCollection services) services.AddAuthorization(); services.AddSingleton(); + services.AddSingleton(); services.AddControllers(); } From 22e0a77685a343e647e73401245fc119d554e4e3 Mon Sep 17 00:00:00 2001 From: Sean Leonard Date: Mon, 18 Jul 2022 13:35:44 -0700 Subject: [PATCH 24/24] added commit for `input_argument_name` = item. --- .../Mutations/MutationBuilder.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/DataGateway.Service.GraphQLBuilder/Mutations/MutationBuilder.cs b/DataGateway.Service.GraphQLBuilder/Mutations/MutationBuilder.cs index 964aa269c8..1bc423b81d 100644 --- a/DataGateway.Service.GraphQLBuilder/Mutations/MutationBuilder.cs +++ b/DataGateway.Service.GraphQLBuilder/Mutations/MutationBuilder.cs @@ -8,6 +8,12 @@ namespace Azure.DataGateway.Service.GraphQLBuilder.Mutations { public static class MutationBuilder { + /// + /// Within a mutation operation, item represents the field holding the metadata + /// used to mutate the underlying database object record. + /// The item field's metadata is of type OperationEntityInput + /// i.e. CreateBookInput + /// public const string INPUT_ARGUMENT_NAME = "item"; ///