diff --git a/Directory.Build.props b/Directory.Build.props index 0e4100e..fbc5773 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -1,7 +1,7 @@ - 5.0.0-preview + 6.0.0-preview 12.0 Shane Krueger Shane Krueger @@ -20,12 +20,13 @@ true enable false - [7.0.0,8.0.0) + + 8.0.0-preview-1068 $(NoWarn);IDE0056;IDE0057;NU1902;NU1903 - + diff --git a/README.md b/README.md index eb2db69..6347f61 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ [![NuGet](https://img.shields.io/nuget/v/GraphQL.AspNetCore3.svg)](https://www.nuget.org/packages/GraphQL.AspNetCore3) [![Coverage Status](https://coveralls.io/repos/github/Shane32/GraphQL.AspNetCore3/badge.svg?branch=master)](https://coveralls.io/github/Shane32/GraphQL.AspNetCore3?branch=master) -This package is designed for ASP.Net Core (2.1 through 6.0) to facilitate easy set-up of GraphQL requests +This package is designed for ASP.Net Core (2.1 through 8.0) to facilitate easy set-up of GraphQL requests over HTTP. The code is designed to be used as middleware within the ASP.Net Core pipeline, serving GET, POST or WebSocket requests. GET requests process requests from the querystring. POST requests can be in the form of JSON requests, form submissions, or raw GraphQL strings. diff --git a/migration.md b/migration.md index 79b56bd..2c145ad 100644 --- a/migration.md +++ b/migration.md @@ -1,8 +1,54 @@ # Version history / migration notes +## 6.0.0 + +GraphQL.AspNetCore3 v6 requires GraphQL.NET v8 or newer. + +### New features + +- When using `FormFileGraphType` with type-first schemas, you may specify the allowed media + types for the file by using the new `[MediaType]` attribute on the argument or input object field. +- Cross-site request forgery (CSRF) protection has been added for both GET and POST requests, + enabled by default. +- Status codes for validation errors are now, by default, determined by the response content type, + and for authentication errors may return a 401 or 403 status code. These changes are purusant + to the [GraphQL over HTTP specification](https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md). + See the breaking changes section below for more information. + +### Breaking changes + +- `GraphQLHttpMiddlewareOptions.ValidationErrorsReturnBadRequest` is now a nullable boolean where + `null` means "use the default behavior". The default behavior is to return a 200 status code + when the response content type is `application/json` and a 400 status code otherwise. The + default value for this in v7 was `true`; set this option to retain the v7 behavior. +- The validation rules' signatures have changed slightly due to the underlying changes to the + GraphQL.NET library. Please see the GraphQL.NET v8 migration document for more information. +- Cross-site request forgery (CSRF) protection has been enabled for all requests by default. + This will require that the `GraphQL-Require-Preflight` header be sent with all GET requests and + all form-POST requests. To disable this feature, set the `CsrfProtectionEnabled` property on the + `GraphQLMiddlewareOptions` class to `false`. You may also configure the headers list by modifying + the `CsrfProtectionHeaders` property on the same class. See the readme for more details. +- Form POST requests are disabled by default; to enable them, set the `ReadFormOnPost` setting + to `true`. +- Validation errors such as authentication errors may now be returned with a 'preferred' status + code instead of a 400 status code. This occurs when (1) the response would otherwise contain + a 400 status code (e.g. the execution of the document has not yet begun), and (2) all errors + in the response prefer the same status code. For practical purposes, this means that the included + errors triggered by the authorization validation rule will now return 401 or 403 when appropriate. +- The `SelectResponseContentType` method now returns a `MediaTypeHeaderValue` instead of a string. +- The `AuthorizationVisitorBase.GetRecursivelyReferencedUsedFragments` method has been removed as + `ValidationContext` now provides an overload to `GetRecursivelyReferencedFragments` which will only + return fragments in use by the specified operation. +- The `AuthorizationVisitorBase.SkipNode` method has been removed as `ValidationContext` now provides + a `ShouldIncludeNode` method. + +## Other changes + +- GraphiQL has been bumped from 1.5.1 to 3.2.0. + ## 5.0.0 -GraphQL.AspNetCore3 v5 requires GraphQL.NET v7 or newer. +GraphQL.AspNetCore3 v5 requires GraphQL.NET v7. `builder.AddAuthorization()` has been renamed to `builder.AddAuthorizationRule()`. The old method has been marked as deprecated. diff --git a/src/GraphQL.AspNetCore3/AuthorizationValidationRule.cs b/src/GraphQL.AspNetCore3/AuthorizationValidationRule.cs index d06243f..6b74d68 100644 --- a/src/GraphQL.AspNetCore3/AuthorizationValidationRule.cs +++ b/src/GraphQL.AspNetCore3/AuthorizationValidationRule.cs @@ -7,10 +7,10 @@ namespace GraphQL.AspNetCore3; /// /// Validates a document against the configured set of policy and role requirements. /// -public class AuthorizationValidationRule : IValidationRule +public class AuthorizationValidationRule : ValidationRuleBase { /// - public virtual async ValueTask ValidateAsync(ValidationContext context) + public override async ValueTask GetPreNodeVisitorAsync(ValidationContext context) { var user = context.User ?? throw new InvalidOperationException("User could not be retrieved from ValidationContext. Please be sure it is set in ExecutionOptions.User."); diff --git a/src/GraphQL.AspNetCore3/AuthorizationVisitorBase.Validation.cs b/src/GraphQL.AspNetCore3/AuthorizationVisitorBase.Validation.cs index d581c30..cc5a9e5 100644 --- a/src/GraphQL.AspNetCore3/AuthorizationVisitorBase.Validation.cs +++ b/src/GraphQL.AspNetCore3/AuthorizationVisitorBase.Validation.cs @@ -16,7 +16,7 @@ public virtual ValueTask ValidateSchemaAsync(ValidationContext context) /// /// Validate a node that is current within the context. /// - private ValueTask ValidateAsync(IProvideMetadata obj, ASTNode? node, ValidationContext context) + private ValueTask ValidateAsync(IMetadataReader obj, ASTNode? node, ValidationContext context) => ValidateAsync(BuildValidationInfo(node, obj, context)); /// @@ -25,7 +25,7 @@ private ValueTask ValidateAsync(IProvideMetadata obj, ASTNode? node, Valid /// The specified . /// The , or which has been matched to the node specified in . /// The validation context. - private static ValidationInfo BuildValidationInfo(ASTNode? node, IProvideMetadata obj, ValidationContext context) + private static ValidationInfo BuildValidationInfo(ASTNode? node, IMetadataReader obj, ValidationContext context) { IFieldType? parentFieldType = null; IGraphType? parentGraphType = null; @@ -50,7 +50,7 @@ private static ValidationInfo BuildValidationInfo(ASTNode? node, IProvideMetadat /// For graph types other than operations, the field where this type was referenced; for query arguments, the field to which this argument belongs. /// For graph types, the graph type for the field where this type was referenced; for field types, the graph type to which this field belongs; for query arguments, the graph type for the field to which this argument belongs. public readonly record struct ValidationInfo( - IProvideMetadata Obj, + IMetadataReader Obj, ASTNode? Node, IFieldType? ParentFieldType, IGraphType? ParentGraphType, @@ -63,7 +63,7 @@ public readonly record struct ValidationInfo( /// /// Validates authorization rules for the specified schema, graph, field or query argument. - /// Does not consider + /// Does not consider /// as this is handled elsewhere. /// Returns a value indicating if validation was successful for this node. /// diff --git a/src/GraphQL.AspNetCore3/HttpGetValidationRule.cs b/src/GraphQL.AspNetCore3/HttpGetValidationRule.cs index bb20b62..88f3bc9 100644 --- a/src/GraphQL.AspNetCore3/HttpGetValidationRule.cs +++ b/src/GraphQL.AspNetCore3/HttpGetValidationRule.cs @@ -3,10 +3,10 @@ namespace GraphQL.AspNetCore3; /// /// Validates that HTTP GET requests only execute queries; not mutations or subscriptions. /// -public sealed class HttpGetValidationRule : IValidationRule +public sealed class HttpGetValidationRule : ValidationRuleBase { /// - public ValueTask ValidateAsync(ValidationContext context) + public override ValueTask GetPreNodeVisitorAsync(ValidationContext context) { if (context.Operation.Operation != OperationType.Query) { context.ReportError(new HttpMethodValidationError(context.Document.Source, context.Operation, "Only query operations allowed for GET requests.")); diff --git a/src/GraphQL.AspNetCore3/HttpPostValidationRule.cs b/src/GraphQL.AspNetCore3/HttpPostValidationRule.cs index 2b6ee7c..6907ba8 100644 --- a/src/GraphQL.AspNetCore3/HttpPostValidationRule.cs +++ b/src/GraphQL.AspNetCore3/HttpPostValidationRule.cs @@ -3,10 +3,10 @@ namespace GraphQL.AspNetCore3; /// /// Validates that HTTP POST requests do not execute subscriptions. /// -public sealed class HttpPostValidationRule : IValidationRule +public sealed class HttpPostValidationRule : ValidationRuleBase { /// - public ValueTask ValidateAsync(ValidationContext context) + public override ValueTask GetPreNodeVisitorAsync(ValidationContext context) { if (context.Operation.Operation == OperationType.Subscription) { context.ReportError(new HttpMethodValidationError(context.Document.Source, context.Operation, "Subscription operations are not supported for POST requests.")); diff --git a/src/Tests.ApiApprovals/GraphQL.AspNetCore3.approved.txt b/src/Tests.ApiApprovals/GraphQL.AspNetCore3.approved.txt index 9db0348..7d92ab9 100644 --- a/src/Tests.ApiApprovals/GraphQL.AspNetCore3.approved.txt +++ b/src/Tests.ApiApprovals/GraphQL.AspNetCore3.approved.txt @@ -15,10 +15,10 @@ namespace GraphQL.AspNetCore3 public System.Func? OnNotAuthorizedPolicy { get; set; } public System.Func? OnNotAuthorizedRole { get; set; } } - public class AuthorizationValidationRule : GraphQL.Validation.IValidationRule + public class AuthorizationValidationRule : GraphQL.Validation.ValidationRuleBase { public AuthorizationValidationRule() { } - public virtual System.Threading.Tasks.ValueTask ValidateAsync(GraphQL.Validation.ValidationContext context) { } + public override System.Threading.Tasks.ValueTask GetPreNodeVisitorAsync(GraphQL.Validation.ValidationContext context) { } } public class AuthorizationVisitor : GraphQL.AspNetCore3.AuthorizationVisitorBase { @@ -47,12 +47,12 @@ namespace GraphQL.AspNetCore3 public virtual System.Threading.Tasks.ValueTask ValidateSchemaAsync(GraphQL.Validation.ValidationContext context) { } public readonly struct ValidationInfo : System.IEquatable { - public ValidationInfo(GraphQL.Types.IProvideMetadata Obj, GraphQLParser.AST.ASTNode? Node, GraphQL.Types.IFieldType? ParentFieldType, GraphQL.Types.IGraphType? ParentGraphType, GraphQL.Validation.ValidationContext Context) { } - public GraphQL.Validation.ValidationContext Context { get; set; } - public GraphQLParser.AST.ASTNode? Node { get; set; } - public GraphQL.Types.IProvideMetadata Obj { get; set; } - public GraphQL.Types.IFieldType? ParentFieldType { get; set; } - public GraphQL.Types.IGraphType? ParentGraphType { get; set; } + public ValidationInfo(GraphQL.Types.IMetadataReader Obj, GraphQLParser.AST.ASTNode? Node, GraphQL.Types.IFieldType? ParentFieldType, GraphQL.Types.IGraphType? ParentGraphType, GraphQL.Validation.ValidationContext Context) { } + public GraphQL.Validation.ValidationContext Context { get; init; } + public GraphQLParser.AST.ASTNode? Node { get; init; } + public GraphQL.Types.IMetadataReader Obj { get; init; } + public GraphQL.Types.IFieldType? ParentFieldType { get; init; } + public GraphQL.Types.IGraphType? ParentGraphType { get; init; } } } public class ExecutionResultActionResult : Microsoft.AspNetCore.Mvc.IActionResult @@ -183,15 +183,15 @@ namespace GraphQL.AspNetCore3 public GraphQLHttpMiddleware(Microsoft.AspNetCore.Http.RequestDelegate next, GraphQL.IGraphQLTextSerializer serializer, GraphQL.IDocumentExecuter documentExecuter, Microsoft.Extensions.DependencyInjection.IServiceScopeFactory serviceScopeFactory, GraphQL.AspNetCore3.GraphQLHttpMiddlewareOptions options, Microsoft.Extensions.Hosting.IHostApplicationLifetime hostApplicationLifetime) { } protected override System.Threading.Tasks.ValueTask> BuildUserContextAsync(Microsoft.AspNetCore.Http.HttpContext context, object? payload) { } } - public sealed class HttpGetValidationRule : GraphQL.Validation.IValidationRule + public sealed class HttpGetValidationRule : GraphQL.Validation.ValidationRuleBase { public HttpGetValidationRule() { } - public System.Threading.Tasks.ValueTask ValidateAsync(GraphQL.Validation.ValidationContext context) { } + public override System.Threading.Tasks.ValueTask GetPreNodeVisitorAsync(GraphQL.Validation.ValidationContext context) { } } - public sealed class HttpPostValidationRule : GraphQL.Validation.IValidationRule + public sealed class HttpPostValidationRule : GraphQL.Validation.ValidationRuleBase { public HttpPostValidationRule() { } - public System.Threading.Tasks.ValueTask ValidateAsync(GraphQL.Validation.ValidationContext context) { } + public override System.Threading.Tasks.ValueTask GetPreNodeVisitorAsync(GraphQL.Validation.ValidationContext context) { } } public interface IAuthorizationOptions { diff --git a/src/Tests.ApiApprovals/Tests.ApiTests.csproj b/src/Tests.ApiApprovals/Tests.ApiTests.csproj index 1d333eb..fcbb7d5 100644 --- a/src/Tests.ApiApprovals/Tests.ApiTests.csproj +++ b/src/Tests.ApiApprovals/Tests.ApiTests.csproj @@ -5,10 +5,10 @@ - - - - + + + + runtime; build; native; contentfiles; analyzers; buildtransitive all diff --git a/src/Tests/AuthorizationTests.cs b/src/Tests/AuthorizationTests.cs index cac6b0d..33b7326 100644 --- a/src/Tests/AuthorizationTests.cs +++ b/src/Tests/AuthorizationTests.cs @@ -51,7 +51,7 @@ private IValidationResult Validate(string query, bool shouldPassCoreRules = true var inputs = new GraphQLSerializer().Deserialize(variables) ?? Inputs.Empty; var validator = new DocumentValidator(); - var (coreRulesResult, _) = validator.ValidateAsync(new ValidationOptions { + var validationResult = validator.ValidateAsync(new ValidationOptions { Document = document, Extensions = Inputs.Empty, Operation = (GraphQLOperationDefinition)document.Definitions.First(x => x.Kind == ASTNodeKind.OperationDefinition), @@ -61,9 +61,9 @@ private IValidationResult Validate(string query, bool shouldPassCoreRules = true RequestServices = mockServices.Object, User = _principal, }).GetAwaiter().GetResult(); // there is no async code being tested - coreRulesResult.IsValid.ShouldBe(shouldPassCoreRules); + validationResult.IsValid.ShouldBe(shouldPassCoreRules); - var (result, _) = validator.ValidateAsync(new ValidationOptions { + var result = validator.ValidateAsync(new ValidationOptions { Document = document, Extensions = Inputs.Empty, Operation = (GraphQLOperationDefinition)document.Definitions.First(x => x.Kind == ASTNodeKind.OperationDefinition), @@ -495,7 +495,7 @@ public void UnusedFragmentsAreIgnored() ret.IsValid.ShouldBeFalse(); } - private void Apply(IProvideMetadata obj, Mode mode) + private void Apply(IMetadataWriter obj, Mode mode) { switch (mode) { case Mode.None: @@ -576,7 +576,7 @@ public void MiscErrors(bool noClaimsPrincipal, bool noRequestServices, bool noAu } [Fact] - public void NullIdentity() + public async Task NullIdentity() { var mockAuthorizationService = new Mock(MockBehavior.Strict); var mockServices = new Mock(MockBehavior.Strict); @@ -585,7 +585,7 @@ public void NullIdentity() var validator = new DocumentValidator(); _schema.Authorize(); - var (result, _) = validator.ValidateAsync(new ValidationOptions { + var result = await validator.ValidateAsync(new ValidationOptions { Document = document, Extensions = Inputs.Empty, Operation = (GraphQLOperationDefinition)document.Definitions.Single(x => x.Kind == ASTNodeKind.OperationDefinition), @@ -595,7 +595,7 @@ public void NullIdentity() Variables = Inputs.Empty, RequestServices = mockServices.Object, User = new ClaimsPrincipal(), - }).GetAwaiter().GetResult(); // there is no async code being tested + }); result.Errors.ShouldHaveSingleItem().ShouldBeOfType().Message.ShouldBe("Access denied for schema."); } @@ -665,7 +665,7 @@ public void WithInterface(bool interfaceRequiresAuth, bool queryRequiresAuth, bo [InlineData(@"{ parent { child(invalid: true) } }", null, true)] [InlineData(@"query { ...frag1 }", null, true)] [InlineData(@"query { ...frag1 } fragment frag1 on QueryType { ...frag1 }", null, true)] - public void TestDefective(string query, string variables, bool expectedIsValid) + public void TestDefective(string query, string? variables, bool expectedIsValid) { _query.AddField(new FieldType { Name = "test", Type = typeof(StringGraphType) }).Authorize(); diff --git a/src/Tests/BuilderMethodTests.cs b/src/Tests/BuilderMethodTests.cs index eb61e08..c0cd92b 100644 --- a/src/Tests/BuilderMethodTests.cs +++ b/src/Tests/BuilderMethodTests.cs @@ -235,6 +235,7 @@ public async Task EndpointRouting(string pattern, string url) await VerifyAsync(url); } + [Fact] public async Task EndpointRouting_Invalid() { _hostBuilder.ConfigureServices(services => services.AddRouting()); diff --git a/src/Tests/ChatTests.cs b/src/Tests/ChatTests.cs index 876f527..3cfcfc6 100644 --- a/src/Tests/ChatTests.cs +++ b/src/Tests/ChatTests.cs @@ -83,7 +83,7 @@ public async Task AllMessages() str.ShouldBe("{\"data\":{\"allMessages\":[{\"id\":\"1\",\"message\":\"hello\"},{\"id\":\"2\",\"message\":\"hello\"}]}}"); } - public async Task AddMessageInternal(int id, string name = "John Doe") + private async Task AddMessageInternal(int id, string name = "John Doe") { var str = await _app.ExecutePost( "/graphql", @@ -106,7 +106,7 @@ public async Task DeleteMessage() str.ShouldBe("{\"data\":{\"count\":1}}"); } - public async Task DeleteMessageInternal(int id) + private async Task DeleteMessageInternal(int id) { var str = await _app.ExecutePost( "/graphql", diff --git a/src/Tests/Middleware/AuthorizationTests.cs b/src/Tests/Middleware/AuthorizationTests.cs index 6cece03..d94036e 100644 --- a/src/Tests/Middleware/AuthorizationTests.cs +++ b/src/Tests/Middleware/AuthorizationTests.cs @@ -29,6 +29,7 @@ private TestServer CreateServer(Action? configureServices = .AddAutoSchema() .AddErrorInfoProvider(new CustomErrorInfoProvider(this)) .AddSystemTextJson()); + services.AddRouting(); services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) .AddJwtBearer(options => { options.TokenValidationParameters.ValidateIssuerSigningKey = false; diff --git a/src/Tests/Tests.csproj b/src/Tests/Tests.csproj index fb7d70f..cd5c035 100644 --- a/src/Tests/Tests.csproj +++ b/src/Tests/Tests.csproj @@ -1,17 +1,21 @@ - netcoreapp2.1;netcoreapp3.1;net5.0;net6.0 + netcoreapp2.1;netcoreapp3.1;net5.0;net6.0;net8.0 - net48;netcoreapp2.1;netcoreapp3.1;net5.0;net6.0 + net48;netcoreapp2.1;netcoreapp3.1;net5.0;net6.0;net8.0 + + + + true - - - - + + + + runtime; build; native; contentfiles; analyzers; buildtransitive all @@ -46,13 +50,21 @@ - + + + + 8.0.* + + + 8.0.* + + 6.0.* diff --git a/src/Tests/WebSockets/NewSubscriptionServerTests.cs b/src/Tests/WebSockets/NewSubscriptionServerTests.cs index 6ec1c3f..d26394b 100644 --- a/src/Tests/WebSockets/NewSubscriptionServerTests.cs +++ b/src/Tests/WebSockets/NewSubscriptionServerTests.cs @@ -225,7 +225,7 @@ public async Task OnPongAsync() [InlineData("abc")] [InlineData("")] [InlineData(null)] - public async Task OnSubscribe(string id) + public async Task OnSubscribe(string? id) { var message = new OperationMessage() { Id = id }; _mockServer.Protected().Setup("OnSubscribeAsync", message).CallBase().Verifiable(); @@ -241,7 +241,7 @@ public async Task OnSubscribe(string id) [InlineData("abc")] [InlineData("")] [InlineData(null)] - public async Task OnComplete(string id) + public async Task OnComplete(string? id) { var message = new OperationMessage() { Id = id }; _mockServer.Protected().Setup("OnCompleteAsync", message).CallBase().Verifiable(); diff --git a/src/Tests/WebSockets/OldSubscriptionServerTests.cs b/src/Tests/WebSockets/OldSubscriptionServerTests.cs index 9b35894..3a3907b 100644 --- a/src/Tests/WebSockets/OldSubscriptionServerTests.cs +++ b/src/Tests/WebSockets/OldSubscriptionServerTests.cs @@ -183,7 +183,7 @@ public async Task OnConnectionAcknowledgeAsync() [InlineData("abc")] [InlineData("")] [InlineData(null)] - public async Task OnStart(string id) + public async Task OnStart(string? id) { var message = new OperationMessage() { Id = id }; _mockServer.Protected().Setup("OnStartAsync", message).CallBase().Verifiable(); @@ -199,7 +199,7 @@ public async Task OnStart(string id) [InlineData("abc")] [InlineData("")] [InlineData(null)] - public async Task OnStop(string id) + public async Task OnStop(string? id) { var message = new OperationMessage() { Id = id }; _mockServer.Protected().Setup("OnStopAsync", message).CallBase().Verifiable();