Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/codecov.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# https://docs.codecov.com/docs/codecov-yaml
comment:
behavior: new
2 changes: 1 addition & 1 deletion .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/BasicSample/BasicSample.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="GraphQL.SystemTextJson" Version="4.2.0" />
<PackageReference Include="GraphQL.SystemTextJson" Version="5.0.0-preview-362" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="5.0.1" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ namespace GraphQL.Authorization
public class AuthorizationValidationRule : GraphQL.Validation.IValidationRule
{
public AuthorizationValidationRule(GraphQL.Authorization.IAuthorizationEvaluator evaluator) { }
public System.Threading.Tasks.Task<GraphQL.Validation.INodeVisitor> ValidateAsync(GraphQL.Validation.ValidationContext context) { }
public System.Threading.Tasks.ValueTask<GraphQL.Validation.INodeVisitor> ValidateAsync(GraphQL.Validation.ValidationContext context) { }
}
public class ClaimAuthorizationRequirement : GraphQL.Authorization.IAuthorizationRequirement
{
Expand Down
119 changes: 69 additions & 50 deletions src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,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<string, string>
config.Query = @"query { post }";
config.Schema = BasicSchema();
config.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{ "Admin", "true" }
});
Expand All @@ -29,10 +29,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();
});
}

Expand All @@ -42,11 +42,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<string, string>
config.Query = @"query { post }";
config.Schema = BasicSchema();
config.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{ "Admin", "true" }
});
Expand All @@ -58,10 +58,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();
});
}

Expand All @@ -70,11 +70,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<string, string>
config.Query = @"query { post }";
config.Schema = NestedSchema();
config.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{ "Admin", "true" }
});
Expand All @@ -86,10 +86,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();
});
}

Expand All @@ -98,10 +98,26 @@ 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(string operationName, string query)
{
Settings.AddPolicy("PostPolicy", builder => builder.RequireClaim("admin"));

ShouldPassRule(config =>
{
config.OperationName = operationName;
config.Query = query;
config.Schema = NestedSchema();
});
}

Expand All @@ -110,10 +126,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();
});
}

Expand All @@ -122,11 +138,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<string, string>
config.Query = @"query { author(input: { name: ""Quinn"" }) }";
config.Schema = TypedSchema();
config.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{ "Admin", "true" }
});
Expand All @@ -138,10 +154,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();
});
}

Expand All @@ -152,11 +168,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<string, string>
config.Query = @"query { author(input: { name: ""Quinn"" }) project(input: { name: ""TEST"" }) }";
config.Schema = TypedSchema();
config.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{ "Admin", "true" }
});
Expand All @@ -166,11 +182,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<string, string>
config.Query = @"query { unknown(obj: {id: 7}) }";
config.Schema = TypedSchema();
config.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{ "Admin", "true" }
});
Expand All @@ -182,11 +198,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<string, string>
config.Query = @"query { posts { items { id } } }";
config.Schema = TypedSchema();
config.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{ "Admin", "true" }
});
Expand All @@ -198,11 +214,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();
});
}

Expand Down Expand Up @@ -233,6 +249,7 @@ type Query {
post(id: ID!): Post
posts: [Post]
postsNonNull: [Post!]!
comment: String
}

type Post {
Expand All @@ -256,6 +273,8 @@ public class NestedQueryWithAttributes
public IEnumerable<Post> Posts() => null;

public IEnumerable<Post> PostsNonNull() => null;

public string Comment() => null;
}

[GraphQLAuthorize("PostPolicy")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

<PropertyGroup>
<TargetFrameworks>net5;netcoreapp3.1</TargetFrameworks>
<GraphQLTestVersion>4.2.0</GraphQLTestVersion>
<GraphQLTestVersion>5.0.0-preview-362</GraphQLTestVersion>
</PropertyGroup>

<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/GraphQL.Authorization.Tests/ValidationTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> claims = null)
Expand Down
2 changes: 2 additions & 0 deletions src/GraphQL.Authorization.Tests/ValidationTestConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
1 change: 1 addition & 0 deletions src/GraphQL.Authorization.sln
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 38 additions & 5 deletions src/GraphQL.Authorization/AuthorizationValidationRule.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System;
using System.Linq;
using System.Threading.Tasks;
using GraphQL.Language.AST;
using GraphQL.Types;
Expand All @@ -22,8 +24,35 @@ public AuthorizationValidationRule(IAuthorizationEvaluator evaluator)
_evaluator = evaluator;
}

private bool ShouldBeSkipped(ValidationContext context)
{
if (context.Document.Operations.Count <= 1)
{
return false;
}

var actualOperation = context.Document.Operations.FirstOrDefault(x => x.Name == context.OperationName) ?? context.Document.Operations.FirstOrDefault();

var i = 0;
do
{
try
{
// TODO: rewrite
if (context.TypeInfo.GetAncestor(i++) == actualOperation)
{
return false;
}
}
catch (InvalidOperationException)
{
return true;
}
} while (true);
}

/// <inheritdoc />
public Task<INodeVisitor> ValidateAsync(ValidationContext context)
public ValueTask<INodeVisitor> ValidateAsync(ValidationContext context)
{
var userContext = context.UserContext as IProvideClaimsPrincipal;
var operationType = OperationType.Query;
Expand All @@ -33,10 +62,14 @@ public Task<INodeVisitor> ValidateAsync(ValidationContext context)
// 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<INodeVisitor>(new NodeVisitors(
new MatchingNodeVisitor<Operation>((astType, context) =>
{
if (context.Document.Operations.Count > 1 && astType.Name != context.OperationName)
{
return;
}

operationType = astType.OperationType;

var type = context.TypeInfo.GetLastType();
Expand All @@ -45,7 +78,7 @@ public Task<INodeVisitor> ValidateAsync(ValidationContext context)

new MatchingNodeVisitor<ObjectField>((objectFieldAst, context) =>
{
if (context.TypeInfo.GetArgument()?.ResolvedType.GetNamedType() is IComplexGraphType argumentType)
if (context.TypeInfo.GetArgument()?.ResolvedType.GetNamedType() is IComplexGraphType argumentType && !ShouldBeSkipped(context))
{
var fieldType = argumentType.GetField(objectFieldAst.Name);
CheckAuth(objectFieldAst, fieldType, userContext, context, operationType);
Expand All @@ -56,7 +89,7 @@ public Task<INodeVisitor> ValidateAsync(ValidationContext context)
{
var fieldDef = context.TypeInfo.GetFieldDef();

if (fieldDef == null)
if (fieldDef == null || ShouldBeSkipped(context))
return;

// check target field
Expand Down
Loading