diff --git a/.github/codecov.yaml b/.github/codecov.yaml new file mode 100644 index 0000000..877c249 --- /dev/null +++ b/.github/codecov.yaml @@ -0,0 +1,3 @@ +# https://docs.codecov.com/docs/codecov-yaml +comment: + behavior: new diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index c22c60a..51bf5c3 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -32,7 +32,7 @@ jobs: - name: Install dependencies working-directory: src - run: dotnet restore -p:GraphQLTestVersion=4.4.0 + run: dotnet restore -p:GraphQLTestVersion=5.0.0-preview-362 - name: Build solution working-directory: src diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6c10299..8f99326 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -30,8 +30,7 @@ jobs: - ubuntu-latest - windows-latest graphqlversion: - - 4.2.0 - - 4.4.0 + - 5.0.0-preview-362 steps: - name: Checkout source uses: actions/checkout@v2 diff --git a/src/BasicSample/BasicSample.csproj b/src/BasicSample/BasicSample.csproj index 82eddcb..33fd571 100644 --- a/src/BasicSample/BasicSample.csproj +++ b/src/BasicSample/BasicSample.csproj @@ -11,7 +11,7 @@ - + diff --git a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt index e532611..2c38741 100644 --- a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt +++ b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt @@ -55,7 +55,7 @@ namespace GraphQL.Authorization public class AuthorizationValidationRule : GraphQL.Validation.IValidationRule { public AuthorizationValidationRule(GraphQL.Authorization.IAuthorizationEvaluator evaluator) { } - public System.Threading.Tasks.Task ValidateAsync(GraphQL.Validation.ValidationContext context) { } + public System.Threading.Tasks.ValueTask ValidateAsync(GraphQL.Validation.ValidationContext context) { } } public class ClaimAuthorizationRequirement : GraphQL.Authorization.IAuthorizationRequirement { diff --git a/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs index 1619f2d..fbd36a9 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; using GraphQL.Types; using GraphQL.Types.Relay.DataObjects; using Xunit; @@ -13,11 +14,11 @@ public void class_policy_success() Settings.AddPolicy("ClassPolicy", builder => builder.RequireClaim("admin")); Settings.AddPolicy("FieldPolicy", builder => builder.RequireClaim("admin")); - ShouldPassRule(_ => + ShouldPassRule(config => { - _.Query = @"query { post }"; - _.Schema = BasicSchema(); - _.User = CreatePrincipal(claims: new Dictionary + config.Query = @"query { post }"; + config.Schema = BasicSchema(); + config.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); @@ -29,10 +30,10 @@ public void class_policy_fail() { Settings.AddPolicy("ClassPolicy", builder => builder.RequireClaim("admin")); - ShouldFailRule(_ => + ShouldFailRule(config => { - _.Query = @"query { post }"; - _.Schema = BasicSchema(); + config.Query = @"query { post }"; + config.Schema = BasicSchema(); }); } @@ -42,11 +43,11 @@ public void field_policy_success() Settings.AddPolicy("ClassPolicy", builder => builder.RequireClaim("admin")); Settings.AddPolicy("FieldPolicy", builder => builder.RequireClaim("admin")); - ShouldPassRule(_ => + ShouldPassRule(config => { - _.Query = @"query { post }"; - _.Schema = BasicSchema(); - _.User = CreatePrincipal(claims: new Dictionary + config.Query = @"query { post }"; + config.Schema = BasicSchema(); + config.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); @@ -58,10 +59,10 @@ public void field_policy_fail() { Settings.AddPolicy("FieldPolicy", builder => builder.RequireClaim("admin")); - ShouldFailRule(_ => + ShouldFailRule(config => { - _.Query = @"query { post }"; - _.Schema = BasicSchema(); + config.Query = @"query { post }"; + config.Schema = BasicSchema(); }); } @@ -70,11 +71,11 @@ public void nested_type_policy_success() { Settings.AddPolicy("PostPolicy", builder => builder.RequireClaim("admin")); - ShouldPassRule(_ => + ShouldPassRule(config => { - _.Query = @"query { post }"; - _.Schema = NestedSchema(); - _.User = CreatePrincipal(claims: new Dictionary + config.Query = @"query { post }"; + config.Schema = NestedSchema(); + config.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); @@ -86,10 +87,10 @@ public void nested_type_policy_fail() { Settings.AddPolicy("PostPolicy", builder => builder.RequireClaim("admin")); - ShouldFailRule(_ => + ShouldFailRule(config => { - _.Query = @"query { post }"; - _.Schema = NestedSchema(); + config.Query = @"query { post }"; + config.Schema = NestedSchema(); }); } @@ -98,10 +99,55 @@ public void nested_type_list_policy_fail() { Settings.AddPolicy("PostPolicy", builder => builder.RequireClaim("admin")); - ShouldFailRule(_ => + ShouldFailRule(config => { - _.Query = @"query { posts }"; - _.Schema = NestedSchema(); + config.Query = @"query { posts }"; + config.Schema = NestedSchema(); + }); + } + + // https://github.com/graphql-dotnet/authorization/issues/5 + [Theory] + [InlineData("c", "query p { posts } query c { comment }")] + [InlineData(null, "query c { comment } query p { posts }")] + public void issue5_should_pass(string operationName, string query) + { + Settings.AddPolicy("PostPolicy", builder => builder.RequireClaim("admin")); + + ShouldPassRule(config => + { + config.OperationName = operationName; + config.Query = query; + config.Schema = NestedSchema(); + }); + } + + // https://github.com/graphql-dotnet/authorization/issues/5 + [Theory] + [InlineData("query a { article { id } } query b { article { ...frag } } fragment frag on Article { content }")] + [InlineData("query a { article { ...frag1 author } } query b { article { ...frag2 } } fragment frag1 on Article { id } fragment frag2 on Article { content }")] + public void issue5_with_fragment_should_pass(string query) + { + Settings.AddPolicy("AdminPolicy", builder => builder.RequireClaim("admin")); + + ShouldPassRule(config => + { + config.Query = query; + config.Schema = TypedSchema(); + }); + } + + // https://github.com/graphql-dotnet/authorization/issues/5 + [Fact] + public void issue5_with_fragment_should_fail() + { + Settings.AddPolicy("AdminPolicy", builder => builder.RequireClaim("admin")); + + ShouldFailRule(config => + { + config.Query = "query a { article { ...frag } } query b { article { ...frag } } fragment frag on Article { content }"; + config.Schema = TypedSchema(); + config.ValidateResult = result => result.Errors.Single(x => x.Message == $"You are not authorized to run this query.\nRequired claim 'admin' is not present."); }); } @@ -110,10 +156,10 @@ public void nested_type_list_non_null_policy_fail() { Settings.AddPolicy("PostPolicy", builder => builder.RequireClaim("admin")); - ShouldFailRule(_ => + ShouldFailRule(config => { - _.Query = @"query { postsNonNull }"; - _.Schema = NestedSchema(); + config.Query = @"query { postsNonNull }"; + config.Schema = NestedSchema(); }); } @@ -122,11 +168,11 @@ public void passes_with_claim_on_input_type() { Settings.AddPolicy("FieldPolicy", builder => builder.RequireClaim("admin")); - ShouldPassRule(_ => + ShouldPassRule(config => { - _.Query = @"query { author(input: { name: ""Quinn"" }) }"; - _.Schema = TypedSchema(); - _.User = CreatePrincipal(claims: new Dictionary + config.Query = @"query { author(input: { name: ""Quinn"" }) }"; + config.Schema = TypedSchema(); + config.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); @@ -138,10 +184,10 @@ public void fails_on_missing_claim_on_input_type() { Settings.AddPolicy("FieldPolicy", builder => builder.RequireClaim("admin")); - ShouldFailRule(_ => + ShouldFailRule(config => { - _.Query = @"query { author(input: { name: ""Quinn"" }) }"; - _.Schema = TypedSchema(); + config.Query = @"query { author(input: { name: ""Quinn"" }) }"; + config.Schema = TypedSchema(); }); } @@ -152,11 +198,11 @@ public void passes_with_multiple_policies_on_field_and_single_on_input_type() Settings.AddPolicy("AdminPolicy", builder => builder.RequireClaim("admin")); Settings.AddPolicy("ConfidentialPolicy", builder => builder.RequireClaim("admin")); - ShouldPassRule(_ => + ShouldPassRule(config => { - _.Query = @"query { author(input: { name: ""Quinn"" }) project(input: { name: ""TEST"" }) }"; - _.Schema = TypedSchema(); - _.User = CreatePrincipal(claims: new Dictionary + config.Query = @"query { author(input: { name: ""Quinn"" }) project(input: { name: ""TEST"" }) }"; + config.Schema = TypedSchema(); + config.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); @@ -166,11 +212,11 @@ public void passes_with_multiple_policies_on_field_and_single_on_input_type() [Fact] public void Issue61() { - ShouldPassRule(_ => + ShouldPassRule(config => { - _.Query = @"query { unknown(obj: {id: 7}) }"; - _.Schema = TypedSchema(); - _.User = CreatePrincipal(claims: new Dictionary + config.Query = @"query { unknown(obj: {id: 7}) }"; + config.Schema = TypedSchema(); + config.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); @@ -182,11 +228,11 @@ public void passes_with_policy_on_connection_type() { Settings.AddPolicy("ConnectionPolicy", _ => _.RequireClaim("admin")); - ShouldPassRule(_ => + ShouldPassRule(config => { - _.Query = @"query { posts { items { id } } }"; - _.Schema = TypedSchema(); - _.User = CreatePrincipal(claims: new Dictionary + config.Query = @"query { posts { items { id } } }"; + config.Schema = TypedSchema(); + config.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); @@ -198,11 +244,11 @@ public void fails_on_missing_claim_on_connection_type() { Settings.AddPolicy("ConnectionPolicy", _ => _.RequireClaim("admin")); - ShouldFailRule(_ => + ShouldFailRule(config => { - _.Query = @"query { posts { items { id } } }"; - _.Schema = TypedSchema(); - _.User = CreatePrincipal(); + config.Query = @"query { posts { items { id } } }"; + config.Schema = TypedSchema(); + config.User = CreatePrincipal(); }); } @@ -233,6 +279,7 @@ type Query { post(id: ID!): Post posts: [Post] postsNonNull: [Post!]! + comment: String } type Post { @@ -256,6 +303,8 @@ public class NestedQueryWithAttributes public IEnumerable Posts() => null; public IEnumerable PostsNonNull() => null; + + public string Comment() => null; } [GraphQLAuthorize("PostPolicy")] @@ -272,6 +321,25 @@ public PostGraphType() } } + public class Article + { + public string Id { get; set; } + + public string Author { get; set; } + + public string Content { get; set; } + } + + public class ArticleGraphType : ObjectGraphType
+ { + public ArticleGraphType() + { + Field(p => p.Id); + Field(p => p.Author); + Field(p => p.Content).AuthorizeWith("AdminPolicy"); + } + } + public class Author { public string Name { get; set; } @@ -297,6 +365,11 @@ private static ISchema TypedSchema() resolve: context => "testing" ).AuthorizeWith("AdminPolicy").AuthorizeWith("ConfidentialPolicy"); + query.Field( + "article", + resolve: context => null + ); + return new Schema { Query = query }; } diff --git a/src/GraphQL.Authorization.Tests/GraphQL.Authorization.Tests.csproj b/src/GraphQL.Authorization.Tests/GraphQL.Authorization.Tests.csproj index f7a6c63..abc4a6f 100644 --- a/src/GraphQL.Authorization.Tests/GraphQL.Authorization.Tests.csproj +++ b/src/GraphQL.Authorization.Tests/GraphQL.Authorization.Tests.csproj @@ -3,7 +3,7 @@ net5;netcoreapp3.1 - 4.2.0 + 5.0.0-preview-362 diff --git a/src/GraphQL.Authorization.Tests/ValidationTestBase.cs b/src/GraphQL.Authorization.Tests/ValidationTestBase.cs index 10efddf..4e09d9c 100644 --- a/src/GraphQL.Authorization.Tests/ValidationTestBase.cs +++ b/src/GraphQL.Authorization.Tests/ValidationTestBase.cs @@ -63,7 +63,7 @@ private static IValidationResult Validate(ValidationTestConfig config) var documentBuilder = new GraphQLDocumentBuilder(); var document = documentBuilder.Build(config.Query); var validator = new DocumentValidator(); - return validator.ValidateAsync(config.Schema, document, document.Operations.First().Variables, config.Rules, userContext, config.Inputs).GetAwaiter().GetResult().validationResult; + return validator.ValidateAsync(config.Schema, document, document.Operations.First().Variables, config.Rules, userContext, config.Inputs, config.OperationName).GetAwaiter().GetResult().validationResult; } internal static ClaimsPrincipal CreatePrincipal(string authenticationType = null, IDictionary claims = null) diff --git a/src/GraphQL.Authorization.Tests/ValidationTestConfig.cs b/src/GraphQL.Authorization.Tests/ValidationTestConfig.cs index 9dec7b4..ea34dbc 100644 --- a/src/GraphQL.Authorization.Tests/ValidationTestConfig.cs +++ b/src/GraphQL.Authorization.Tests/ValidationTestConfig.cs @@ -8,6 +8,8 @@ namespace GraphQL.Authorization.Tests { public class ValidationTestConfig { + public string OperationName { get; set; } + public string Query { get; set; } public ISchema Schema { get; set; } diff --git a/src/GraphQL.Authorization.sln b/src/GraphQL.Authorization.sln index 8f48539..b29d818 100644 --- a/src/GraphQL.Authorization.sln +++ b/src/GraphQL.Authorization.sln @@ -21,6 +21,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".github", ".github", "{84514A09-9BB4-4C85-8A8E-92AF5AA26445}" ProjectSection(SolutionItems) = preProject + ..\.github\codecov.yaml = ..\.github\codecov.yaml ..\.github\dependabot.yml = ..\.github\dependabot.yml ..\.github\FUNDING.yml = ..\.github\FUNDING.yml ..\.github\labeler.yml = ..\.github\labeler.yml diff --git a/src/GraphQL.Authorization/AuthorizationValidationRule.cs b/src/GraphQL.Authorization/AuthorizationValidationRule.cs index 892c542..a9dc909 100644 --- a/src/GraphQL.Authorization/AuthorizationValidationRule.cs +++ b/src/GraphQL.Authorization/AuthorizationValidationRule.cs @@ -1,3 +1,4 @@ +using System.Linq; using System.Threading.Tasks; using GraphQL.Language.AST; using GraphQL.Types; @@ -22,21 +23,78 @@ public AuthorizationValidationRule(IAuthorizationEvaluator evaluator) _evaluator = evaluator; } + private bool ShouldBeSkipped(Operation actualOperation, ValidationContext context) + { + if (context.Document.Operations.Count <= 1) + { + return false; + } + + var i = 0; + do + { + var ancestor = context.TypeInfo.GetAncestor(i++); + + if (ancestor == actualOperation) + { + return false; + } + + if (ancestor == context.Document) + { + return true; + } + + if (ancestor is FragmentDefinition fragment) + { + return !FragmentBelongsToOperation(fragment, actualOperation); + } + } while (true); + } + + private bool FragmentBelongsToOperation(FragmentDefinition fragment, Operation operation) + { + var belongs = false; + void Visit(INode node, int _) + { + if (belongs) + { + return; + } + + belongs = node is FragmentSpread fragmentSpread && fragmentSpread.Name == fragment.Name; + + if (node != null) + { + node.Visit(Visit, 0); + } + } + + operation.Visit(Visit, 0); + + return belongs; + } + /// - public Task ValidateAsync(ValidationContext context) + public ValueTask ValidateAsync(ValidationContext context) { var userContext = context.UserContext as IProvideClaimsPrincipal; var operationType = OperationType.Query; + var actualOperation = context.Document.Operations.FirstOrDefault(x => x.Name == context.OperationName) ?? context.Document.Operations.FirstOrDefault(); // this could leak info about hidden fields or types in error messages // it would be better to implement a filter on the Schema so it // acts as if they just don't exist vs. an auth denied error // - filtering the Schema is not currently supported // TODO: apply ISchemaFilter - context.Schema.Filter.AllowXXX - - return Task.FromResult((INodeVisitor)new NodeVisitors( + return new ValueTask(new NodeVisitors( new MatchingNodeVisitor((astType, context) => { + if (context.Document.Operations.Count > 1 && astType.Name != context.OperationName) + { + return; + } + operationType = astType.OperationType; var type = context.TypeInfo.GetLastType(); @@ -45,7 +103,7 @@ public Task ValidateAsync(ValidationContext context) new MatchingNodeVisitor((objectFieldAst, context) => { - if (context.TypeInfo.GetArgument()?.ResolvedType.GetNamedType() is IComplexGraphType argumentType) + if (context.TypeInfo.GetArgument()?.ResolvedType.GetNamedType() is IComplexGraphType argumentType && !ShouldBeSkipped(actualOperation, context)) { var fieldType = argumentType.GetField(objectFieldAst.Name); CheckAuth(objectFieldAst, fieldType, userContext, context, operationType); @@ -56,7 +114,7 @@ public Task ValidateAsync(ValidationContext context) { var fieldDef = context.TypeInfo.GetFieldDef(); - if (fieldDef == null) + if (fieldDef == null || ShouldBeSkipped(actualOperation, context)) return; // check target field diff --git a/src/GraphQL.Authorization/GraphQL.Authorization.csproj b/src/GraphQL.Authorization/GraphQL.Authorization.csproj index ccd33b9..8ad5c6e 100644 --- a/src/GraphQL.Authorization/GraphQL.Authorization.csproj +++ b/src/GraphQL.Authorization/GraphQL.Authorization.csproj @@ -7,7 +7,7 @@ - +