From 8e7cb3d4c0e453a2221896208962278983af61c3 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Mon, 14 Jan 2019 23:58:34 +0100 Subject: [PATCH 1/6] Started fixing the auth directive --- .../AuthorizeDirective.cs | 89 ++++++++++++++++++- .../AuthorizeDirectiveType.cs | 23 +++-- 2 files changed, 106 insertions(+), 6 deletions(-) diff --git a/src/Server/AspNetCore.Authorization/AuthorizeDirective.cs b/src/Server/AspNetCore.Authorization/AuthorizeDirective.cs index 64a43ed8f6a..636fee9634e 100644 --- a/src/Server/AspNetCore.Authorization/AuthorizeDirective.cs +++ b/src/Server/AspNetCore.Authorization/AuthorizeDirective.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Runtime.Serialization; +using HotChocolate.Language; #if !ASPNETCLASSIC using System.Collections.ObjectModel; @@ -13,7 +15,12 @@ namespace HotChocolate.AspNetCore.Authorization #endif { public class AuthorizeDirective + : ISerializable { + public AuthorizeDirective() + { + } + #if ASPNETCLASSIC public AuthorizeDirective(IEnumerable roles) { @@ -21,9 +28,40 @@ public AuthorizeDirective(IEnumerable roles) { throw new ArgumentNullException(nameof(roles)); } - + Roles = roles.ToArray(); } + + public AuthorizeDirective( + SerializationInfo info, + StreamingContext context) + { + var node = info.GetValue( + nameof(DirectiveNode), + typeof(DirectiveNode)) + as DirectiveNode; + + if (node == null) + { + Roles = (string[])info.GetValue( + nameof(Roles), + typeof(string[])); + } + else + { + ArgumentNode rolesArgument = node.Arguments + .FirstOrDefault(t => t.Name.Value == "roles"); + + Roles = (rolesArgument != null + && rolesArgument.Value is ListValueNode lv) + ? lv.Items.OfType() + .Select(t => t.Value?.Trim()) + .Where(s => !string.IsNullOrEmpty(s)) + .ToArray() + : Array.Empty(); + } + } + #else public AuthorizeDirective(string policy) : this(policy, null) @@ -49,6 +87,44 @@ public AuthorizeDirective(string policy, IEnumerable roles) Roles = readOnlyRoles; } + public AuthorizeDirective( + SerializationInfo info, + StreamingContext context) + { + var node = info.GetValue( + nameof(DirectiveNode), + typeof(DirectiveNode)) + as DirectiveNode; + + if (node == null) + { + Policy = info.GetString(nameof(Policy)); + Roles = (string[])info.GetValue( + nameof(Roles), + typeof(string[])); + } + else + { + ArgumentNode policyArgument = node.Arguments + .FirstOrDefault(t => t.Name.Value == "policy"); + ArgumentNode rolesArgument = node.Arguments + .FirstOrDefault(t => t.Name.Value == "roles"); + + Policy = (policyArgument != null + && policyArgument.Value is StringValueNode sv) + ? sv.Value + : null; + + Roles = (rolesArgument != null + && rolesArgument.Value is ListValueNode lv) + ? lv.Items.OfType() + .Select(t => t.Value?.Trim()) + .Where(s => !string.IsNullOrEmpty(s)) + .ToArray() + : Array.Empty(); + } + } + /// /// Gets or sets the policy name that determines access to the resource. /// @@ -59,5 +135,16 @@ public AuthorizeDirective(string policy, IEnumerable roles) /// Gets or sets of roles that are allowed to access the resource. /// public IReadOnlyCollection Roles { get; } + + + public void GetObjectData( + SerializationInfo info, + StreamingContext context) + { +#if !ASPNETCLASSIC + info.AddValue(nameof(Policy), Policy); +#endif + info.AddValue(nameof(Roles), Roles?.ToArray()); + } } } diff --git a/src/Server/AspNetCore.Authorization/AuthorizeDirectiveType.cs b/src/Server/AspNetCore.Authorization/AuthorizeDirectiveType.cs index 8b154ef523e..29489655f9c 100644 --- a/src/Server/AspNetCore.Authorization/AuthorizeDirectiveType.cs +++ b/src/Server/AspNetCore.Authorization/AuthorizeDirectiveType.cs @@ -38,18 +38,31 @@ private static async Task AuthorizeAsync( #if !ASPNETCLASSIC IAuthorizationService authorizeService = context .Service(); + IAuthorizationPolicyProvider policyProvider = context + .Service(); #endif ClaimsPrincipal principal = context .CustomProperty(nameof(ClaimsPrincipal)); AuthorizeDirective directive = context.Directive .ToObject(); - bool allowed = IsInRoles(principal, directive.Roles); + var allowed = IsInRoles(principal, directive.Roles); #if !ASPNETCLASSIC - if (allowed && !string.IsNullOrEmpty(directive.Policy)) + if (allowed) { - AuthorizationResult result = await authorizeService - .AuthorizeAsync(principal, directive.Policy); + AuthorizationPolicy policy = + string.IsNullOrWhiteSpace(directive.Policy) + ? await policyProvider.GetDefaultPolicyAsync() + : await policyProvider.GetPolicyAsync(directive.Policy); + + if (policy == null) + { + context.Result = QueryErro + return; + } + + AuthorizationResult result = + await authorizeService.AuthorizeAsync(principal, policy); allowed = result.Succeeded; } @@ -75,7 +88,7 @@ private static bool IsInRoles( { if (roles != null) { - foreach (string role in roles) + foreach (var role in roles) { if (!principal.IsInRole(role)) { From 899e1586a4da1ed554cd13e53cc7e6368df90528 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Tue, 15 Jan 2019 08:02:53 +0100 Subject: [PATCH 2/6] Reworked auth directive --- .../AuthorizeDirectiveType.cs | 91 +++++++++++++------ 1 file changed, 61 insertions(+), 30 deletions(-) diff --git a/src/Server/AspNetCore.Authorization/AuthorizeDirectiveType.cs b/src/Server/AspNetCore.Authorization/AuthorizeDirectiveType.cs index 29489655f9c..e1e7cd2dd95 100644 --- a/src/Server/AspNetCore.Authorization/AuthorizeDirectiveType.cs +++ b/src/Server/AspNetCore.Authorization/AuthorizeDirectiveType.cs @@ -5,14 +5,12 @@ using HotChocolate.Execution; using HotChocolate.Resolvers; using HotChocolate.Types; - -#if !ASPNETCLASSIC -using Microsoft.AspNetCore.Authorization; -#endif - #if ASPNETCLASSIC + namespace HotChocolate.AspNetClassic.Authorization #else +using Microsoft.AspNetCore.Authorization; + namespace HotChocolate.AspNetCore.Authorization #endif { @@ -27,6 +25,8 @@ protected override void Configure( descriptor.Location(DirectiveLocation.Object) .Location(DirectiveLocation.FieldDefinition); + descriptor.Repeatable(); + descriptor.Middleware( next => context => AuthorizeAsync(context, next)); } @@ -35,39 +35,22 @@ private static async Task AuthorizeAsync( IDirectiveContext context, DirectiveDelegate next) { -#if !ASPNETCLASSIC - IAuthorizationService authorizeService = context - .Service(); - IAuthorizationPolicyProvider policyProvider = context - .Service(); -#endif - ClaimsPrincipal principal = context - .CustomProperty(nameof(ClaimsPrincipal)); AuthorizeDirective directive = context.Directive .ToObject(); + + ClaimsPrincipal principal = context + .CustomProperty(nameof(ClaimsPrincipal)); + var allowed = IsInRoles(principal, directive.Roles); #if !ASPNETCLASSIC - if (allowed) + if(allowed) { - AuthorizationPolicy policy = - string.IsNullOrWhiteSpace(directive.Policy) - ? await policyProvider.GetDefaultPolicyAsync() - : await policyProvider.GetPolicyAsync(directive.Policy); - - if (policy == null) - { - context.Result = QueryErro - return; - } - - AuthorizationResult result = - await authorizeService.AuthorizeAsync(principal, policy); - - allowed = result.Succeeded; + allowed = await AuthorizeWithPolicyAsync( + context, directive, principal) + .ConfigureAwait(false); } #endif - if (allowed) { await next(context); @@ -99,5 +82,53 @@ private static bool IsInRoles( return true; } +#if !ASPNETCLASSIC + + private static async Task AuthorizeWithPolicyAsync( + IDirectiveContext context, + AuthorizeDirective directive, + ClaimsPrincipal principal) + { + IAuthorizationService authorizeService = context + .Service(); + IAuthorizationPolicyProvider policyProvider = context + .Service(); + + AuthorizationPolicy policy = null; + + if (directive.Roles == null + && string.IsNullOrWhiteSpace(directive.Policy)) + { + policy = await policyProvider.GetDefaultPolicyAsync(); + if (policy == null) + { + context.Result = QueryError.CreateFieldError( + "The default authorization policy does not exist.", + context.FieldSelection); + } + } + + else if (!string.IsNullOrWhiteSpace(directive.Policy)) + { + policy = await policyProvider.GetPolicyAsync(directive.Policy); + if (policy == null) + { + context.Result = QueryError.CreateFieldError( + $"The `{directive.Policy}` authorization policy " + + "does not exist.", + context.FieldSelection); + } + } + + if (context.Result == null && policy != null) + { + AuthorizationResult result = + await authorizeService.AuthorizeAsync(principal, policy); + return result.Succeeded; + } + + return false; + } +#endif } } From 967433e13c02596f4175ef4c7e390e9984c26289 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Tue, 15 Jan 2019 09:19:40 +0100 Subject: [PATCH 3/6] Added authorization tests for aspnet core --- .../AuthorizeDirective.cs | 6 +- .../AuthorizeDirectiveType.cs | 13 +- .../Authorization/AuthorizationTests.cs | 403 +++++++++++++++++- .../Helpers/ClientQueryResult.cs | 3 +- .../Helpers/TestServerFactory.cs | 8 + .../AspNetCore.Tests/QueryMiddlewareTests.cs | 5 +- .../__snapshots__/DefaultPolicy_NotFound.json | 16 + .../__snapshots__/Policy_Authorized.json | 6 + .../__snapshots__/Policy_NotAuthorized.json | 19 + .../__snapshots__/Policy_NotFound.json | 16 + .../__snapshots__/Roles_Authorized.json | 6 + ...Roles_UserHasAllOfTheRoles_Authorized.json | 6 + ...s_UserHasDifferentRoles_NotAuthorized.json | 19 + .../Roles_UserHasNoRoles_NotAuthorized.json | 19 + ...es_UserHasOneOfTheRoles_NotAuthorized.json | 19 + .../UsePolicy_NotAuthorized.json | 19 + 16 files changed, 564 insertions(+), 19 deletions(-) create mode 100644 src/Server/AspNetCore.Tests/__snapshots__/DefaultPolicy_NotFound.json create mode 100644 src/Server/AspNetCore.Tests/__snapshots__/Policy_Authorized.json create mode 100644 src/Server/AspNetCore.Tests/__snapshots__/Policy_NotAuthorized.json create mode 100644 src/Server/AspNetCore.Tests/__snapshots__/Policy_NotFound.json create mode 100644 src/Server/AspNetCore.Tests/__snapshots__/Roles_Authorized.json create mode 100644 src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasAllOfTheRoles_Authorized.json create mode 100644 src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasDifferentRoles_NotAuthorized.json create mode 100644 src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasNoRoles_NotAuthorized.json create mode 100644 src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasOneOfTheRoles_NotAuthorized.json create mode 100644 src/Server/AspNetCore.Tests/__snapshots__/UsePolicy_NotAuthorized.json diff --git a/src/Server/AspNetCore.Authorization/AuthorizeDirective.cs b/src/Server/AspNetCore.Authorization/AuthorizeDirective.cs index 636fee9634e..bb98e83d12c 100644 --- a/src/Server/AspNetCore.Authorization/AuthorizeDirective.cs +++ b/src/Server/AspNetCore.Authorization/AuthorizeDirective.cs @@ -19,6 +19,7 @@ public class AuthorizeDirective { public AuthorizeDirective() { + Roles = Array.Empty(); } #if ASPNETCLASSIC @@ -77,14 +78,15 @@ public AuthorizeDirective(string policy, IEnumerable roles) roles?.ToList().AsReadOnly(); if (string.IsNullOrEmpty(policy) - && (readOnlyRoles == null || readOnlyRoles.Any())) + && (readOnlyRoles == null || readOnlyRoles.Count > 0)) { throw new ArgumentException( "Either policy or roles has to be set."); } Policy = policy; - Roles = readOnlyRoles; + Roles = (IReadOnlyCollection)readOnlyRoles + ?? Array.Empty(); } public AuthorizeDirective( diff --git a/src/Server/AspNetCore.Authorization/AuthorizeDirectiveType.cs b/src/Server/AspNetCore.Authorization/AuthorizeDirectiveType.cs index e1e7cd2dd95..8b83d88bbfc 100644 --- a/src/Server/AspNetCore.Authorization/AuthorizeDirectiveType.cs +++ b/src/Server/AspNetCore.Authorization/AuthorizeDirectiveType.cs @@ -44,7 +44,7 @@ private static async Task AuthorizeAsync( var allowed = IsInRoles(principal, directive.Roles); #if !ASPNETCLASSIC - if(allowed) + if (allowed && NeedsPolicyValidation(directive)) { allowed = await AuthorizeWithPolicyAsync( context, directive, principal) @@ -55,7 +55,7 @@ private static async Task AuthorizeAsync( { await next(context); } - else + else if (context.Result == null) { context.Result = QueryError.CreateFieldError( "The current user is not authorized to " + @@ -84,6 +84,13 @@ private static bool IsInRoles( } #if !ASPNETCLASSIC + private static bool NeedsPolicyValidation( + AuthorizeDirective directive) + { + return directive.Roles.Count == 0 + || !string.IsNullOrEmpty(directive.Policy); + } + private static async Task AuthorizeWithPolicyAsync( IDirectiveContext context, AuthorizeDirective directive, @@ -96,7 +103,7 @@ private static async Task AuthorizeWithPolicyAsync( AuthorizationPolicy policy = null; - if (directive.Roles == null + if (directive.Roles.Count == 0 && string.IsNullOrWhiteSpace(directive.Policy)) { policy = await policyProvider.GetDefaultPolicyAsync(); diff --git a/src/Server/AspNetCore.Tests/Authorization/AuthorizationTests.cs b/src/Server/AspNetCore.Tests/Authorization/AuthorizationTests.cs index 6ff8e9f13a8..fab7b157c3c 100644 --- a/src/Server/AspNetCore.Tests/Authorization/AuthorizationTests.cs +++ b/src/Server/AspNetCore.Tests/Authorization/AuthorizationTests.cs @@ -1,7 +1,18 @@ -using Microsoft.AspNetCore.Authorization; +using System; +using System.Net; +using System.Net.Http; +using System.Security.Claims; +using System.Threading.Tasks; +using ChilliCream.Testing; +using HotChocolate.Execution; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Newtonsoft.Json; using Xunit; -namespace HotChocolate.AspNetCore.Subscriptions +namespace HotChocolate.AspNetCore.Authorization { public class AuthorizationTests : IClassFixture @@ -12,21 +23,391 @@ public AuthorizationTests(TestServerFactory testServerFactory) } - private TestServerFactory TestServerFactory { get; } + private TestServerFactory TestServerFactory { get; } + + [Fact] + public async Task DefaultPolicy_NotFound() + { + // arrange + TestServer server = CreateTestServer( + services => + { + services.AddAuthorization(options => + { + options.DefaultPolicy = null; + }); + services.AddGraphQL(CreateExecuter()); + }, + context => + { + // no user + }); + + var request = "{ default }"; + var contentType = "application/graphql"; + + // act + HttpResponseMessage message = + await server.SendPostRequestAsync(request, contentType, null); + + // assert + Assert.Equal(HttpStatusCode.OK, message.StatusCode); + + var json = await message.Content.ReadAsStringAsync(); + ClientQueryResult result = JsonConvert + .DeserializeObject(json); + Assert.NotNull(result.Errors); + result.Snapshot(); + } + + [Fact] + public async Task Policy_NotFound() + { + // arrange + TestServer server = CreateTestServer( + services => + { + services.AddAuthorization(options => + { + options.AddPolicy("FooBar", policy => + policy.RequireAssertion(context => + context.User.HasClaim(c => + c.Type == ClaimTypes.DateOfBirth))); + }); + + services.AddGraphQL(CreateExecuter()); + }, + context => + { + // no user + }); + + var request = "{ age }"; + var contentType = "application/graphql"; + + // act + HttpResponseMessage message = + await server.SendPostRequestAsync(request, contentType, null); + + // assert + Assert.Equal(HttpStatusCode.OK, message.StatusCode); + + var json = await message.Content.ReadAsStringAsync(); + ClientQueryResult result = JsonConvert + .DeserializeObject(json); + Assert.NotNull(result.Errors); + result.Snapshot(); + } + + [Fact] + public async Task Policy_NotAuthorized() + { + // arrange + TestServer server = CreateTestServer( + services => + { + services.AddAuthorization(options => + { + options.AddPolicy("HasAgeDefined", policy => + policy.RequireAssertion(context => + context.User.HasClaim(c => + c.Type == ClaimTypes.DateOfBirth))); + }); + + services.AddGraphQL(CreateExecuter()); + }, + context => + { + // no user + }); + + var request = "{ age }"; + var contentType = "application/graphql"; + + // act + HttpResponseMessage message = + await server.SendPostRequestAsync(request, contentType, null); + + // assert + Assert.Equal(HttpStatusCode.OK, message.StatusCode); + + var json = await message.Content.ReadAsStringAsync(); + ClientQueryResult result = JsonConvert + .DeserializeObject(json); + Assert.NotNull(result.Errors); + result.Snapshot(); + } + + [Fact] + public async Task Policy_Authorized() + { + // arrange + TestServer server = CreateTestServer( + services => + { + services.AddAuthorization(options => + { + options.AddPolicy("HasAgeDefined", policy => + policy.RequireAssertion(context => + context.User.HasClaim(c => + c.Type == ClaimTypes.DateOfBirth))); + }); + + services.AddGraphQL(CreateExecuter()); + }, + context => + { + var identity = new ClaimsIdentity(); + identity.AddClaim(new Claim( + ClaimTypes.DateOfBirth, + "2013-05-30")); + context.User.AddIdentity(identity); + }); + + var request = "{ age }"; + var contentType = "application/graphql"; + + // act + HttpResponseMessage message = + await server.SendPostRequestAsync(request, contentType, null); + + // assert + Assert.Equal(HttpStatusCode.OK, message.StatusCode); + + var json = await message.Content.ReadAsStringAsync(); + ClientQueryResult result = JsonConvert + .DeserializeObject(json); + Assert.Null(result.Errors); + result.Snapshot(); + } + + [Fact] + public async Task Roles_UserHasNoRoles_NotAuthorized() + { + // arrange + TestServer server = CreateTestServer( + services => + { + services.AddGraphQL(CreateExecuter()); + }, + context => + { + // no user + }); + + var request = "{ roles }"; + var contentType = "application/graphql"; + + // act + HttpResponseMessage message = + await server.SendPostRequestAsync(request, contentType, null); + + // assert + Assert.Equal(HttpStatusCode.OK, message.StatusCode); + + var json = await message.Content.ReadAsStringAsync(); + ClientQueryResult result = JsonConvert + .DeserializeObject(json); + Assert.NotNull(result.Errors); + result.Snapshot(); + } + [Fact] + public async Task Roles_UserHasDifferentRoles_NotAuthorized() + { + // arrange + TestServer server = CreateTestServer( + services => + { + services.AddGraphQL(CreateExecuter()); + }, + context => + { + var identity = new ClaimsIdentity(); + identity.AddClaim(new Claim( + ClaimTypes.Role, + "b")); + context.User.AddIdentity(identity); + }); + + var request = "{ roles }"; + var contentType = "application/graphql"; + + // act + HttpResponseMessage message = + await server.SendPostRequestAsync(request, contentType, null); + + // assert + Assert.Equal(HttpStatusCode.OK, message.StatusCode); + + var json = await message.Content.ReadAsStringAsync(); + ClientQueryResult result = JsonConvert + .DeserializeObject(json); + Assert.NotNull(result.Errors); + result.Snapshot(); + } + [Fact] + public async Task Roles_UserHasOneOfTheRoles_NotAuthorized() + { + // arrange + TestServer server = CreateTestServer( + services => + { + services.AddGraphQL(CreateExecuter()); + }, + context => + { + var identity = new ClaimsIdentity(); + identity.AddClaim(new Claim( + ClaimTypes.Role, + "a")); + identity.AddClaim(new Claim( + ClaimTypes.Role, + "c")); + context.User.AddIdentity(identity); + }); + + var request = "{ roles_ab }"; + var contentType = "application/graphql"; + + // act + HttpResponseMessage message = + await server.SendPostRequestAsync(request, contentType, null); + + // assert + Assert.Equal(HttpStatusCode.OK, message.StatusCode); + + var json = await message.Content.ReadAsStringAsync(); + ClientQueryResult result = JsonConvert + .DeserializeObject(json); + Assert.NotNull(result.Errors); + result.Snapshot(); + } + + [Fact] + public async Task Roles_UserHasAllOfTheRoles_Authorized() + { + // arrange + TestServer server = CreateTestServer( + services => + { + services.AddGraphQL(CreateExecuter()); + }, + context => + { + var identity = new ClaimsIdentity(); + identity.AddClaim(new Claim( + ClaimTypes.Role, + "a")); + identity.AddClaim(new Claim( + ClaimTypes.Role, + "b")); + context.User.AddIdentity(identity); + }); + + var request = "{ roles_ab }"; + var contentType = "application/graphql"; + + // act + HttpResponseMessage message = + await server.SendPostRequestAsync(request, contentType, null); + + // assert + Assert.Equal(HttpStatusCode.OK, message.StatusCode); + + var json = await message.Content.ReadAsStringAsync(); + ClientQueryResult result = JsonConvert + .DeserializeObject(json); + Assert.Null(result.Errors); + result.Snapshot(); + } + + [Fact] + public async Task Roles_Authorized() + { + // arrange + TestServer server = CreateTestServer( + services => + { + services.AddGraphQL(CreateExecuter()); + }, + context => + { + var identity = new ClaimsIdentity(); + identity.AddClaim(new Claim( + ClaimTypes.Role, + "a")); + context.User.AddIdentity(identity); + }); + + var request = "{ roles }"; + var contentType = "application/graphql"; + + // act + HttpResponseMessage message = + await server.SendPostRequestAsync(request, contentType, null); + + // assert + Assert.Equal(HttpStatusCode.OK, message.StatusCode); + + var json = await message.Content.ReadAsStringAsync(); + ClientQueryResult result = JsonConvert + .DeserializeObject(json); + Assert.Null(result.Errors); + result.Snapshot(); + } - } - public class MinimumAgeRequirement - : IAuthorizationRequirement - { - public int MinimumAge { get; private set; } - - public MinimumAgeRequirement(int minimumAge) + private TestServer CreateTestServer( + Action configureServices, + Action configureUser) { - MinimumAge = minimumAge; + return TestServerFactory.Create( + builder => + { + return builder + .ConfigureServices(configureServices) + .Configure(app => + { + app.UseGraphQL(new QueryMiddlewareOptions + { + OnCreateRequest = (ctx, r, p, ct) => + { + configureUser(ctx); + return Task.CompletedTask; + } + }); + }); + }); + } + + + private static IQueryExecuter CreateExecuter() + { + return Schema.Create( + @" + type Query { + default: String @authorize + age: String @authorize(policy: ""HasAgeDefined"") + roles: String @authorize(roles: [""a""]) + roles_ab: String @authorize(roles: [""a"" ""b""]) + piped: String + @authorize(policy: ""a"") + @authorize(policy: ""b"") + } + ", + configuration => + { + configuration.RegisterAuthorizeDirectiveType(); + configuration.Use(next => context => + { + context.Result = "foo"; + return next.Invoke(context); + }); + }) + .MakeExecutable(); } } } diff --git a/src/Server/AspNetCore.Tests/Helpers/ClientQueryResult.cs b/src/Server/AspNetCore.Tests/Helpers/ClientQueryResult.cs index ee59b2c8b19..72093f786a8 100644 --- a/src/Server/AspNetCore.Tests/Helpers/ClientQueryResult.cs +++ b/src/Server/AspNetCore.Tests/Helpers/ClientQueryResult.cs @@ -1,11 +1,10 @@ using System.Collections.Generic; -using HotChocolate.Execution; namespace HotChocolate.AspNetCore { public class ClientQueryResult { public Dictionary Data { get; set; } - public List Errors { get; set; } + public List> Errors { get; set; } } } diff --git a/src/Server/AspNetCore.Tests/Helpers/TestServerFactory.cs b/src/Server/AspNetCore.Tests/Helpers/TestServerFactory.cs index 1834f964e54..855934987f3 100644 --- a/src/Server/AspNetCore.Tests/Helpers/TestServerFactory.cs +++ b/src/Server/AspNetCore.Tests/Helpers/TestServerFactory.cs @@ -33,7 +33,15 @@ public TestServer Create( }); var server = new TestServer(builder); + _instances.Add(server); + return server; + } + public TestServer Create( + Func configure) + { + IWebHostBuilder builder = configure(new WebHostBuilder()); + var server = new TestServer(builder); _instances.Add(server); return server; } diff --git a/src/Server/AspNetCore.Tests/QueryMiddlewareTests.cs b/src/Server/AspNetCore.Tests/QueryMiddlewareTests.cs index 13fa9d61e59..270dcbc52a8 100644 --- a/src/Server/AspNetCore.Tests/QueryMiddlewareTests.cs +++ b/src/Server/AspNetCore.Tests/QueryMiddlewareTests.cs @@ -47,7 +47,10 @@ public async Task HttpPost_Casing() { // arrange TestServer server = CreateTestServer(); - var request = new ClientQueryRequest { Query = "{ A:basic { B:a } }" }; + var request = new ClientQueryRequest + { + Query = "{ A:basic { B:a } }" + }; // act HttpResponseMessage message = diff --git a/src/Server/AspNetCore.Tests/__snapshots__/DefaultPolicy_NotFound.json b/src/Server/AspNetCore.Tests/__snapshots__/DefaultPolicy_NotFound.json new file mode 100644 index 00000000000..dd03e3f2194 --- /dev/null +++ b/src/Server/AspNetCore.Tests/__snapshots__/DefaultPolicy_NotFound.json @@ -0,0 +1,16 @@ +{ + "Data": { + "default": null + }, + "Errors": [ + { + "message": "The default authorization policy does not exist.", + "locations": [ + { + "line": 1, + "column": 3 + } + ] + } + ] +} diff --git a/src/Server/AspNetCore.Tests/__snapshots__/Policy_Authorized.json b/src/Server/AspNetCore.Tests/__snapshots__/Policy_Authorized.json new file mode 100644 index 00000000000..b0d0fdd18a1 --- /dev/null +++ b/src/Server/AspNetCore.Tests/__snapshots__/Policy_Authorized.json @@ -0,0 +1,6 @@ +{ + "Data": { + "age": "foo" + }, + "Errors": null +} diff --git a/src/Server/AspNetCore.Tests/__snapshots__/Policy_NotAuthorized.json b/src/Server/AspNetCore.Tests/__snapshots__/Policy_NotAuthorized.json new file mode 100644 index 00000000000..6102c8384c7 --- /dev/null +++ b/src/Server/AspNetCore.Tests/__snapshots__/Policy_NotAuthorized.json @@ -0,0 +1,19 @@ +{ + "Data": { + "age": null + }, + "Errors": [ + { + "message": "The current user is not authorized to access this resource.", + "path": [ + "age" + ], + "locations": [ + { + "line": 1, + "column": 3 + } + ] + } + ] +} diff --git a/src/Server/AspNetCore.Tests/__snapshots__/Policy_NotFound.json b/src/Server/AspNetCore.Tests/__snapshots__/Policy_NotFound.json new file mode 100644 index 00000000000..a91b6285072 --- /dev/null +++ b/src/Server/AspNetCore.Tests/__snapshots__/Policy_NotFound.json @@ -0,0 +1,16 @@ +{ + "Data": { + "age": null + }, + "Errors": [ + { + "message": "The `HasAgeDefined` authorization policy does not exist.", + "locations": [ + { + "line": 1, + "column": 3 + } + ] + } + ] +} diff --git a/src/Server/AspNetCore.Tests/__snapshots__/Roles_Authorized.json b/src/Server/AspNetCore.Tests/__snapshots__/Roles_Authorized.json new file mode 100644 index 00000000000..5d6e3a53bfd --- /dev/null +++ b/src/Server/AspNetCore.Tests/__snapshots__/Roles_Authorized.json @@ -0,0 +1,6 @@ +{ + "Data": { + "roles": "foo" + }, + "Errors": null +} diff --git a/src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasAllOfTheRoles_Authorized.json b/src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasAllOfTheRoles_Authorized.json new file mode 100644 index 00000000000..dd5714dbc6d --- /dev/null +++ b/src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasAllOfTheRoles_Authorized.json @@ -0,0 +1,6 @@ +{ + "Data": { + "roles_ab": "foo" + }, + "Errors": null +} diff --git a/src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasDifferentRoles_NotAuthorized.json b/src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasDifferentRoles_NotAuthorized.json new file mode 100644 index 00000000000..519767b38f2 --- /dev/null +++ b/src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasDifferentRoles_NotAuthorized.json @@ -0,0 +1,19 @@ +{ + "Data": { + "roles": null + }, + "Errors": [ + { + "message": "The current user is not authorized to access this resource.", + "path": [ + "roles" + ], + "locations": [ + { + "line": 1, + "column": 3 + } + ] + } + ] +} diff --git a/src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasNoRoles_NotAuthorized.json b/src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasNoRoles_NotAuthorized.json new file mode 100644 index 00000000000..519767b38f2 --- /dev/null +++ b/src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasNoRoles_NotAuthorized.json @@ -0,0 +1,19 @@ +{ + "Data": { + "roles": null + }, + "Errors": [ + { + "message": "The current user is not authorized to access this resource.", + "path": [ + "roles" + ], + "locations": [ + { + "line": 1, + "column": 3 + } + ] + } + ] +} diff --git a/src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasOneOfTheRoles_NotAuthorized.json b/src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasOneOfTheRoles_NotAuthorized.json new file mode 100644 index 00000000000..4aa2726d3c6 --- /dev/null +++ b/src/Server/AspNetCore.Tests/__snapshots__/Roles_UserHasOneOfTheRoles_NotAuthorized.json @@ -0,0 +1,19 @@ +{ + "Data": { + "roles_ab": null + }, + "Errors": [ + { + "message": "The current user is not authorized to access this resource.", + "path": [ + "roles_ab" + ], + "locations": [ + { + "line": 1, + "column": 3 + } + ] + } + ] +} diff --git a/src/Server/AspNetCore.Tests/__snapshots__/UsePolicy_NotAuthorized.json b/src/Server/AspNetCore.Tests/__snapshots__/UsePolicy_NotAuthorized.json new file mode 100644 index 00000000000..6102c8384c7 --- /dev/null +++ b/src/Server/AspNetCore.Tests/__snapshots__/UsePolicy_NotAuthorized.json @@ -0,0 +1,19 @@ +{ + "Data": { + "age": null + }, + "Errors": [ + { + "message": "The current user is not authorized to access this resource.", + "path": [ + "age" + ], + "locations": [ + { + "line": 1, + "column": 3 + } + ] + } + ] +} From f2b5ef8126eea0692321d5b68d760cf608d5f203 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Tue, 15 Jan 2019 09:24:05 +0100 Subject: [PATCH 4/6] Added more tests --- .../Authorization/AuthorizationTests.cs | 98 +++++++++++++++++++ .../PipedAuthorizeDirectives_Authorized.json | 6 ++ ...eDirectives_SecondFails_NotAuthorized.json | 19 ++++ 3 files changed, 123 insertions(+) create mode 100644 src/Server/AspNetCore.Tests/__snapshots__/PipedAuthorizeDirectives_Authorized.json create mode 100644 src/Server/AspNetCore.Tests/__snapshots__/PipedAuthorizeDirectives_SecondFails_NotAuthorized.json diff --git a/src/Server/AspNetCore.Tests/Authorization/AuthorizationTests.cs b/src/Server/AspNetCore.Tests/Authorization/AuthorizationTests.cs index fab7b157c3c..cdca1950295 100644 --- a/src/Server/AspNetCore.Tests/Authorization/AuthorizationTests.cs +++ b/src/Server/AspNetCore.Tests/Authorization/AuthorizationTests.cs @@ -358,7 +358,105 @@ public async Task Roles_Authorized() Assert.Null(result.Errors); result.Snapshot(); } + + [Fact] + public async Task PipedAuthorizeDirectives_Authorized() + { + // arrange + TestServer server = CreateTestServer( + services => + { + services.AddAuthorization(options => + { + options.AddPolicy("a", policy => + policy.RequireAssertion(context => + context.User.HasClaim(c => + c.Type == ClaimTypes.DateOfBirth))); + + options.AddPolicy("b", policy => + policy.RequireAssertion(context => + context.User.HasClaim(c => + c.Type == ClaimTypes.Country))); + }); + + services.AddGraphQL(CreateExecuter()); + }, + context => + { + var identity = new ClaimsIdentity(); + identity.AddClaim(new Claim( + ClaimTypes.DateOfBirth, + "2013-05-30")); + identity.AddClaim(new Claim( + ClaimTypes.Country, + "US")); + context.User.AddIdentity(identity); + }); + + var request = "{ piped }"; + var contentType = "application/graphql"; + + // act + HttpResponseMessage message = + await server.SendPostRequestAsync(request, contentType, null); + + // assert + Assert.Equal(HttpStatusCode.OK, message.StatusCode); + + var json = await message.Content.ReadAsStringAsync(); + ClientQueryResult result = JsonConvert + .DeserializeObject(json); + Assert.Null(result.Errors); + result.Snapshot(); + } + [Fact] + public async Task PipedAuthorizeDirectives_SecondFails_NotAuthorized() + { + // arrange + TestServer server = CreateTestServer( + services => + { + services.AddAuthorization(options => + { + options.AddPolicy("a", policy => + policy.RequireAssertion(context => + context.User.HasClaim(c => + c.Type == ClaimTypes.DateOfBirth))); + + options.AddPolicy("b", policy => + policy.RequireAssertion(context => + context.User.HasClaim(c => + c.Type == ClaimTypes.Country))); + }); + + services.AddGraphQL(CreateExecuter()); + }, + context => + { + var identity = new ClaimsIdentity(); + identity.AddClaim(new Claim( + ClaimTypes.DateOfBirth, + "2013-05-30")); + context.User.AddIdentity(identity); + }); + + var request = "{ piped }"; + var contentType = "application/graphql"; + + // act + HttpResponseMessage message = + await server.SendPostRequestAsync(request, contentType, null); + + // assert + Assert.Equal(HttpStatusCode.OK, message.StatusCode); + + var json = await message.Content.ReadAsStringAsync(); + ClientQueryResult result = JsonConvert + .DeserializeObject(json); + Assert.NotNull(result.Errors); + result.Snapshot(); + } private TestServer CreateTestServer( Action configureServices, diff --git a/src/Server/AspNetCore.Tests/__snapshots__/PipedAuthorizeDirectives_Authorized.json b/src/Server/AspNetCore.Tests/__snapshots__/PipedAuthorizeDirectives_Authorized.json new file mode 100644 index 00000000000..3aca7bf5357 --- /dev/null +++ b/src/Server/AspNetCore.Tests/__snapshots__/PipedAuthorizeDirectives_Authorized.json @@ -0,0 +1,6 @@ +{ + "Data": { + "piped": "foo" + }, + "Errors": null +} diff --git a/src/Server/AspNetCore.Tests/__snapshots__/PipedAuthorizeDirectives_SecondFails_NotAuthorized.json b/src/Server/AspNetCore.Tests/__snapshots__/PipedAuthorizeDirectives_SecondFails_NotAuthorized.json new file mode 100644 index 00000000000..1ef10a59fbb --- /dev/null +++ b/src/Server/AspNetCore.Tests/__snapshots__/PipedAuthorizeDirectives_SecondFails_NotAuthorized.json @@ -0,0 +1,19 @@ +{ + "Data": { + "piped": null + }, + "Errors": [ + { + "message": "The current user is not authorized to access this resource.", + "path": [ + "piped" + ], + "locations": [ + { + "line": 1, + "column": 3 + } + ] + } + ] +} From a00675b2349477f7191c1977957f172a0b571b84 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Tue, 15 Jan 2019 09:37:59 +0100 Subject: [PATCH 5/6] Fixed root type null ref exception --- .../Execution/Errors/ErrorBehaviourTests.cs | 38 ++++++++++++++++++- .../__snapshots__/RootFieldNotDefined.json | 15 ++++++++ .../__snapshots__/RootTypeNotDefined.json | 9 +++++ src/Core/Core/Validation/QueryVisitor.cs | 9 +++-- 4 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 src/Core/Core.Tests/__snapshots__/RootFieldNotDefined.json create mode 100644 src/Core/Core.Tests/__snapshots__/RootTypeNotDefined.json diff --git a/src/Core/Core.Tests/Execution/Errors/ErrorBehaviourTests.cs b/src/Core/Core.Tests/Execution/Errors/ErrorBehaviourTests.cs index 0de346c0903..12e3312718c 100644 --- a/src/Core/Core.Tests/Execution/Errors/ErrorBehaviourTests.cs +++ b/src/Core/Core.Tests/Execution/Errors/ErrorBehaviourTests.cs @@ -8,7 +8,7 @@ namespace HotChocolate.Execution { public class ErrorBehaviourTests { - [Fact] + [Fact] public async Task SyntaxError() { // arrange @@ -231,6 +231,42 @@ public async Task Property_Return_UnexpectedErrorWithPath() result.Snapshot(); } + [Fact] + public async Task RootTypeNotDefined() + { + // arrange + string query = "mutation { foo }"; + + var schema = Schema.Create( + "type Query { foo: String }", + c => c.Use(next => context => Task.CompletedTask)); + IQueryExecuter executer = schema.MakeExecutable(); + + // act + IExecutionResult result = await executer.ExecuteAsync(query); + + // assert + result.Snapshot(); + } + + [Fact] + public async Task RootFieldNotDefined() + { + // arrange + string query = "mutation { foo }"; + + var schema = Schema.Create( + "type Mutation { bar: String }", + c => c.Use(next => context => Task.CompletedTask)); + IQueryExecuter executer = schema.MakeExecutable(); + + // act + IExecutionResult result = await executer.ExecuteAsync(query); + + // assert + result.Snapshot(); + } + private async Task ExecuteQuery( string query, Action errorHandled) diff --git a/src/Core/Core.Tests/__snapshots__/RootFieldNotDefined.json b/src/Core/Core.Tests/__snapshots__/RootFieldNotDefined.json new file mode 100644 index 00000000000..05a66dedf85 --- /dev/null +++ b/src/Core/Core.Tests/__snapshots__/RootFieldNotDefined.json @@ -0,0 +1,15 @@ +{ + "Data": {}, + "Extensions": {}, + "Errors": [ + { + "message": "The field `foo` does not exist on the type `Mutation`.", + "locations": [ + { + "line": 1, + "column": 12 + } + ] + } + ] +} diff --git a/src/Core/Core.Tests/__snapshots__/RootTypeNotDefined.json b/src/Core/Core.Tests/__snapshots__/RootTypeNotDefined.json new file mode 100644 index 00000000000..e5fa8e74cda --- /dev/null +++ b/src/Core/Core.Tests/__snapshots__/RootTypeNotDefined.json @@ -0,0 +1,9 @@ +{ + "Data": {}, + "Extensions": {}, + "Errors": [ + { + "message": "The specified root type `Mutation` does not exist." + } + ] +} diff --git a/src/Core/Core/Validation/QueryVisitor.cs b/src/Core/Core/Validation/QueryVisitor.cs index 296c9becff4..ced0b171125 100644 --- a/src/Core/Core/Validation/QueryVisitor.cs +++ b/src/Core/Core/Validation/QueryVisitor.cs @@ -81,9 +81,12 @@ protected virtual void VisitOperationDefinition( ImmutableStack path) { IType operationType = Schema.GetOperationType(operation.Operation); - ImmutableStack newPath = path.Push(operation); - VisitSelectionSet(operation.SelectionSet, operationType, newPath); - VisitDirectives(operation.Directives, newPath); + if (operationType != null) + { + ImmutableStack newPath = path.Push(operation); + VisitSelectionSet(operation.SelectionSet, operationType, newPath); + VisitDirectives(operation.Directives, newPath); + } _visitedFragments.Clear(); } From b50b405487879777fb7b0e78e6e6fa132cfc643e Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Tue, 15 Jan 2019 09:59:12 +0100 Subject: [PATCH 6/6] Fixed sonar issues --- src/Core/Core/Validation/QueryVisitor.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Core/Core/Validation/QueryVisitor.cs b/src/Core/Core/Validation/QueryVisitor.cs index ced0b171125..84caba37893 100644 --- a/src/Core/Core/Validation/QueryVisitor.cs +++ b/src/Core/Core/Validation/QueryVisitor.cs @@ -84,7 +84,12 @@ protected virtual void VisitOperationDefinition( if (operationType != null) { ImmutableStack newPath = path.Push(operation); - VisitSelectionSet(operation.SelectionSet, operationType, newPath); + + VisitSelectionSet( + operation.SelectionSet, + operationType, + newPath); + VisitDirectives(operation.Directives, newPath); } _visitedFragments.Clear(); @@ -110,7 +115,8 @@ protected virtual void VisitSelectionSet( } else if (selection is InlineFragmentNode inlineFragment) { - VisitInlineFragmentInternal(inlineFragment, type, newpath); + VisitInlineFragmentInternal( + inlineFragment, type, newpath); } } } @@ -255,7 +261,8 @@ protected bool ContainsFragment(string fragmentName) return _fragments.ContainsKey(fragmentName); } - protected bool IsFragmentVisited(FragmentDefinitionNode fragmentDefinition) + protected bool IsFragmentVisited( + FragmentDefinitionNode fragmentDefinition) { if (fragmentDefinition == null) { @@ -265,7 +272,8 @@ protected bool IsFragmentVisited(FragmentDefinitionNode fragmentDefinition) return _visitedFragments.Contains(fragmentDefinition); } - protected bool MarkFragmentVisited(FragmentDefinitionNode fragmentDefinition) + protected bool MarkFragmentVisited( + FragmentDefinitionNode fragmentDefinition) { if (fragmentDefinition == null) {