Skip to content

Commit bdedd6e

Browse files
sungam3rsungam3r
committed
IAuthorizationSkipCondition feature
1 parent 620fcb9 commit bdedd6e

File tree

6 files changed

+207
-4
lines changed

6 files changed

+207
-4
lines changed

‎src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ namespace GraphQL.Authorization
5555
public class AuthorizationValidationRule : GraphQL.Validation.IValidationRule
5656
{
5757
public AuthorizationValidationRule(GraphQL.Authorization.IAuthorizationEvaluator evaluator) { }
58+
public AuthorizationValidationRule(GraphQL.Authorization.IAuthorizationEvaluator evaluator, System.Collections.Generic.IEnumerable<GraphQL.Authorization.IAuthorizationSkipCondition> skipConditions) { }
5859
public System.Threading.Tasks.ValueTask<GraphQL.Validation.INodeVisitor?> ValidateAsync(GraphQL.Validation.ValidationContext context) { }
5960
}
6061
public class ClaimAuthorizationRequirement : GraphQL.Authorization.IAuthorizationRequirement
@@ -80,8 +81,17 @@ namespace GraphQL.Authorization
8081
{
8182
System.Threading.Tasks.Task Authorize(GraphQL.Authorization.AuthorizationContext context);
8283
}
84+
public interface IAuthorizationSkipCondition
85+
{
86+
System.Threading.Tasks.ValueTask<bool> ShouldSkip(GraphQL.Validation.ValidationContext context);
87+
}
8388
public interface IProvideClaimsPrincipal
8489
{
8590
System.Security.Claims.ClaimsPrincipal? User { get; }
8691
}
92+
public class IntrospectionSkipCondition : GraphQL.Authorization.IAuthorizationSkipCondition
93+
{
94+
public IntrospectionSkipCondition() { }
95+
public System.Threading.Tasks.ValueTask<bool> ShouldSkip(GraphQL.Validation.ValidationContext context) { }
96+
}
8797
}

‎src/GraphQL.Authorization.Tests/AuthorizationSkipTests.cs

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
using GraphQL.Types;
2+
using Xunit;
3+
4+
namespace GraphQL.Authorization.Tests
5+
{
6+
/// <summary>
7+
/// Tests for <see cref="IntrospectionSkipCondition"/>.
8+
/// https://github.com/graphql-dotnet/authorization/issues/28
9+
/// </summary>
10+
public class AuthorizationSkipTests : ValidationTestBase
11+
{
12+
[Fact]
13+
public void passes_with_skip_condition()
14+
{
15+
Rule = new AuthorizationValidationRule(new AuthorizationEvaluator(Settings), new[] { new IntrospectionSkipCondition() });
16+
Settings.AddPolicy("AdminPolicy", _ => _.RequireClaim("admin"));
17+
18+
ShouldPassRule(config =>
19+
{
20+
config.Query = QUERY;
21+
config.Schema = CreateSchema();
22+
});
23+
}
24+
25+
[Fact]
26+
public void fails_without_skip_condition()
27+
{
28+
Settings.AddPolicy("AdminPolicy", _ => _.RequireClaim("admin"));
29+
30+
ShouldFailRule(config =>
31+
{
32+
config.Query = QUERY;
33+
config.Schema = CreateSchema();
34+
});
35+
}
36+
37+
[Fact]
38+
public void fails_with_skip_condition_and_extra_fields()
39+
{
40+
Rule = new AuthorizationValidationRule(new AuthorizationEvaluator(Settings), new[] { new IntrospectionSkipCondition() });
41+
Settings.AddPolicy("AdminPolicy", _ => _.RequireClaim("admin"));
42+
43+
ShouldFailRule(config =>
44+
{
45+
config.Query = QUERY.Replace("...frag1", "...frag1 info");
46+
config.Schema = CreateSchema();
47+
});
48+
}
49+
50+
private static ISchema CreateSchema() =>
51+
Schema.For("type Query { info: String! }", builder => builder.Types.Include<Query>());
52+
53+
[GraphQLAuthorize("AdminPolicy")]
54+
public class Query
55+
{
56+
public string Info() => "OK";
57+
}
58+
59+
private const string QUERY = @"
60+
query
61+
{
62+
__typename
63+
__type(name: ""__Schema"")
64+
{
65+
name
66+
description
67+
}
68+
x: __schema
69+
{
70+
queryType
71+
{
72+
name
73+
}
74+
}
75+
...frag1
76+
... on Query
77+
{
78+
inline: __typename
79+
}
80+
}
81+
82+
fragment frag1 on Query
83+
{
84+
s: __typename
85+
}";
86+
}
87+
}

‎src/GraphQL.Authorization.Tests/ValidationTestBase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public ValidationTestBase()
1616
Rule = new AuthorizationValidationRule(new AuthorizationEvaluator(Settings));
1717
}
1818

19-
protected AuthorizationValidationRule Rule { get; }
19+
protected AuthorizationValidationRule Rule { get; set; }
2020

2121
protected AuthorizationSettings Settings { get; }
2222

‎src/GraphQL.Authorization/AuthorizationValidationRule.cs

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Collections.Generic;
23
using System.Linq;
34
using System.Threading.Tasks;
@@ -14,14 +15,25 @@ namespace GraphQL.Authorization
1415
public class AuthorizationValidationRule : IValidationRule
1516
{
1617
private readonly IAuthorizationEvaluator _evaluator;
18+
private readonly IAuthorizationSkipCondition[] _skipConditions;
1719

1820
/// <summary>
1921
/// Creates an instance of <see cref="AuthorizationValidationRule"/> with
2022
/// the specified authorization evaluator.
2123
/// </summary>
2224
public AuthorizationValidationRule(IAuthorizationEvaluator evaluator)
25+
: this(evaluator, null!)
26+
{
27+
}
28+
29+
/// <summary>
30+
/// Creates an instance of <see cref="AuthorizationValidationRule"/> with
31+
/// the specified authorization evaluator and authorization skip conditions.
32+
/// </summary>
33+
public AuthorizationValidationRule(IAuthorizationEvaluator evaluator, IEnumerable<IAuthorizationSkipCondition> skipConditions)
2334
{
2435
_evaluator = evaluator;
36+
_skipConditions = skipConditions?.ToArray() ?? Array.Empty<IAuthorizationSkipCondition>();
2537
}
2638

2739
private bool ShouldBeSkipped(Operation actualOperation, ValidationContext context)
@@ -77,8 +89,25 @@ void Visit(INode node, int _)
7789
}
7890

7991
/// <inheritdoc />
80-
public ValueTask<INodeVisitor?> ValidateAsync(ValidationContext context)
92+
public async ValueTask<INodeVisitor?> ValidateAsync(ValidationContext context)
8193
{
94+
async ValueTask<bool> ShouldSkipAuthorization(ValidationContext context)
95+
{
96+
if (_skipConditions.Length == 0)
97+
return false;
98+
99+
foreach (var skipCondition in _skipConditions)
100+
{
101+
if (!await skipCondition.ShouldSkip(context))
102+
return false;
103+
}
104+
105+
return true;
106+
}
107+
108+
if (await ShouldSkipAuthorization(context))
109+
return null;
110+
82111
var userContext = context.UserContext as IProvideClaimsPrincipal;
83112
var operationType = OperationType.Query;
84113
var actualOperation = context.Document.Operations.FirstOrDefault(x => x.Name == context.OperationName) ?? context.Document.Operations.FirstOrDefault();
@@ -88,14 +117,15 @@ void Visit(INode node, int _)
88117
// acts as if they just don't exist vs. an auth denied error
89118
// - filtering the Schema is not currently supported
90119
// TODO: apply ISchemaFilter - context.Schema.Filter.AllowXXX
91-
return new ValueTask<INodeVisitor?>(new NodeVisitors(
120+
return new NodeVisitors(
92121
new MatchingNodeVisitor<Operation>((astType, context) =>
93122
{
94123
if (context.Document.Operations.Count > 1 && astType.Name != context.OperationName)
95124
{
96125
return;
97126
}
98127

128+
// Actually, astType always equals actualOperation
99129
operationType = astType.OperationType;
100130

101131
var type = context.TypeInfo.GetLastType();
@@ -148,7 +178,7 @@ void Visit(INode node, int _)
148178
}
149179
}
150180
})
151-
));
181+
);
152182
}
153183

154184
private void CheckAuth(

‎src/GraphQL.Authorization/IAuthorizationSkipCondition.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
using System.Threading.Tasks;
2+
using GraphQL.Validation;
3+
4+
namespace GraphQL.Authorization
5+
{
6+
/// <summary>
7+
/// Allows to conditionally skip entire AST traversing and all
8+
/// authorization checks in <see cref="AuthorizationValidationRule"/>.
9+
/// </summary>
10+
public interface IAuthorizationSkipCondition
11+
{
12+
/// <summary>
13+
/// Specifies whether authorization checks should be skipped.
14+
/// </summary>
15+
ValueTask<bool> ShouldSkip(ValidationContext context);
16+
}
17+
}

‎src/GraphQL.Authorization/IntrospectionSkipCondition.cs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
using System.Linq;
2+
using System.Threading.Tasks;
3+
using GraphQL.Language.AST;
4+
using GraphQL.Validation;
5+
6+
namespace GraphQL.Authorization
7+
{
8+
/// <summary>
9+
/// Skips authorization checks for introspection queries, namely all queries
10+
/// that contain only __schema, __type and __typename top-level fields.
11+
/// </summary>
12+
public class IntrospectionSkipCondition : IAuthorizationSkipCondition
13+
{
14+
/// <inheritdoc />
15+
public ValueTask<bool> ShouldSkip(ValidationContext context)
16+
{
17+
static bool IsIntrospectionField(Field f) => f.Name == "__schema" || f.Name == "__type" || f.Name == "__typename";
18+
19+
bool ContainsOnlyIntrospectionFields(IHaveSelectionSet node)
20+
{
21+
if (node.SelectionSet?.Selections?.Count == 0)
22+
return false; // invalid document, better to return false
23+
24+
foreach (var selection in node.SelectionSet!.Selections)
25+
{
26+
switch (selection)
27+
{
28+
case Field field:
29+
if (!IsIntrospectionField(field))
30+
return false;
31+
break;
32+
33+
case InlineFragment inlineFragment:
34+
if (!ContainsOnlyIntrospectionFields(inlineFragment))
35+
return false;
36+
break;
37+
38+
case FragmentSpread fragmentSpread:
39+
var fragmentDef = context.Document.Fragments.FindDefinition(fragmentSpread.Name);
40+
if (fragmentDef == null || !ContainsOnlyIntrospectionFields(fragmentDef))
41+
return false;
42+
break;
43+
44+
default:
45+
return false;
46+
}
47+
}
48+
49+
return true;
50+
}
51+
52+
var actualOperation = context.Document.Operations.FirstOrDefault(x => x.Name == context.OperationName) ?? context.Document.Operations.FirstOrDefault();
53+
54+
return new ValueTask<bool>(actualOperation?.OperationType == OperationType.Query
55+
? ContainsOnlyIntrospectionFields(actualOperation)
56+
: false); // not an executable document
57+
}
58+
}
59+
}

0 commit comments

Comments
 (0)