Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IAuthorizationSkipCondition feature #186

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ namespace GraphQL.Authorization
public class AuthorizationValidationRule : GraphQL.Validation.IValidationRule
{
public AuthorizationValidationRule(GraphQL.Authorization.IAuthorizationEvaluator evaluator) { }
public AuthorizationValidationRule(GraphQL.Authorization.IAuthorizationEvaluator evaluator, System.Collections.Generic.IEnumerable<GraphQL.Authorization.IAuthorizationSkipCondition> skipConditions) { }
public System.Threading.Tasks.ValueTask<GraphQL.Validation.INodeVisitor?> ValidateAsync(GraphQL.Validation.ValidationContext context) { }
}
public class ClaimAuthorizationRequirement : GraphQL.Authorization.IAuthorizationRequirement
Expand All @@ -80,8 +81,17 @@ namespace GraphQL.Authorization
{
System.Threading.Tasks.Task Authorize(GraphQL.Authorization.AuthorizationContext context);
}
public interface IAuthorizationSkipCondition
{
System.Threading.Tasks.ValueTask<bool> ShouldSkip(GraphQL.Validation.ValidationContext context);
}
public interface IProvideClaimsPrincipal
{
System.Security.Claims.ClaimsPrincipal? User { get; }
}
public class IntrospectionSkipCondition : GraphQL.Authorization.IAuthorizationSkipCondition
{
public IntrospectionSkipCondition() { }
public System.Threading.Tasks.ValueTask<bool> ShouldSkip(GraphQL.Validation.ValidationContext context) { }
}
}
87 changes: 87 additions & 0 deletions src/GraphQL.Authorization.Tests/AuthorizationSkipTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
using GraphQL.Types;
using Xunit;

namespace GraphQL.Authorization.Tests
{
/// <summary>
/// Tests for <see cref="IntrospectionSkipCondition"/>.
/// https://github.com/graphql-dotnet/authorization/issues/28
/// </summary>
public class AuthorizationSkipTests : ValidationTestBase
{
[Fact]
public void passes_with_skip_condition()
{
Rule = new AuthorizationValidationRule(new AuthorizationEvaluator(Settings), new[] { new IntrospectionSkipCondition() });
Settings.AddPolicy("AdminPolicy", _ => _.RequireClaim("admin"));

ShouldPassRule(config =>
{
config.Query = QUERY;
config.Schema = CreateSchema();
});
}

[Fact]
public void fails_without_skip_condition()
{
Settings.AddPolicy("AdminPolicy", _ => _.RequireClaim("admin"));

ShouldFailRule(config =>
{
config.Query = QUERY;
config.Schema = CreateSchema();
});
}

[Fact]
public void fails_with_skip_condition_and_extra_fields()
{
Rule = new AuthorizationValidationRule(new AuthorizationEvaluator(Settings), new[] { new IntrospectionSkipCondition() });
Settings.AddPolicy("AdminPolicy", _ => _.RequireClaim("admin"));

ShouldFailRule(config =>
{
config.Query = QUERY.Replace("...frag1", "...frag1 info");
config.Schema = CreateSchema();
});
}

private static ISchema CreateSchema() =>
Schema.For("type Query { info: String! }", builder => builder.Types.Include<Query>());

[GraphQLAuthorize("AdminPolicy")]
public class Query
{
public string Info() => "OK";
}

private const string QUERY = @"
query
{
__typename
__type(name: ""__Schema"")
{
name
description
}
x: __schema
{
queryType
{
name
}
}
...frag1
... on Query
{
inline: __typename
}
}

fragment frag1 on Query
{
s: __typename
}";
}
}
2 changes: 1 addition & 1 deletion src/GraphQL.Authorization.Tests/ValidationTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public ValidationTestBase()
Rule = new AuthorizationValidationRule(new AuthorizationEvaluator(Settings));
}

protected AuthorizationValidationRule Rule { get; }
protected AuthorizationValidationRule Rule { get; set; }

protected AuthorizationSettings Settings { get; }

Expand Down
36 changes: 33 additions & 3 deletions src/GraphQL.Authorization/AuthorizationValidationRule.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
Expand All @@ -14,14 +15,25 @@ namespace GraphQL.Authorization
public class AuthorizationValidationRule : IValidationRule
{
private readonly IAuthorizationEvaluator _evaluator;
private readonly IAuthorizationSkipCondition[] _skipConditions;

/// <summary>
/// Creates an instance of <see cref="AuthorizationValidationRule"/> with
/// the specified authorization evaluator.
/// </summary>
public AuthorizationValidationRule(IAuthorizationEvaluator evaluator)
: this(evaluator, null!)
{
}

/// <summary>
/// Creates an instance of <see cref="AuthorizationValidationRule"/> with
/// the specified authorization evaluator and authorization skip conditions.
/// </summary>
public AuthorizationValidationRule(IAuthorizationEvaluator evaluator, IEnumerable<IAuthorizationSkipCondition> skipConditions)
{
_evaluator = evaluator;
_skipConditions = skipConditions?.ToArray() ?? Array.Empty<IAuthorizationSkipCondition>();
}

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

/// <inheritdoc />
public ValueTask<INodeVisitor?> ValidateAsync(ValidationContext context)
public async ValueTask<INodeVisitor?> ValidateAsync(ValidationContext context)
{
async ValueTask<bool> ShouldSkipAuthorization(ValidationContext context)
{
if (_skipConditions.Length == 0)
return false;

foreach (var skipCondition in _skipConditions)
{
if (!await skipCondition.ShouldSkip(context))
return false;
}

return true;
}

if (await ShouldSkipAuthorization(context))
return null;

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();
Expand All @@ -88,14 +117,15 @@ void Visit(INode node, int _)
// 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 new ValueTask<INodeVisitor?>(new NodeVisitors(
return new NodeVisitors(
new MatchingNodeVisitor<Operation>((astType, context) =>
{
if (context.Document.Operations.Count > 1 && astType.Name != context.OperationName)
{
return;
}

// Actually, astType always equals actualOperation
operationType = astType.OperationType;

var type = context.TypeInfo.GetLastType();
Expand Down Expand Up @@ -148,7 +178,7 @@ void Visit(INode node, int _)
}
}
})
));
);
}

private void CheckAuth(
Expand Down
17 changes: 17 additions & 0 deletions src/GraphQL.Authorization/IAuthorizationSkipCondition.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System.Threading.Tasks;
using GraphQL.Validation;

namespace GraphQL.Authorization
{
/// <summary>
/// Allows to conditionally skip entire AST traversing and all
/// authorization checks in <see cref="AuthorizationValidationRule"/>.
/// </summary>
public interface IAuthorizationSkipCondition
{
/// <summary>
/// Specifies whether authorization checks should be skipped.
/// </summary>
ValueTask<bool> ShouldSkip(ValidationContext context);
}
}
59 changes: 59 additions & 0 deletions src/GraphQL.Authorization/IntrospectionSkipCondition.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
using System.Linq;
using System.Threading.Tasks;
using GraphQL.Language.AST;
using GraphQL.Validation;

namespace GraphQL.Authorization
{
/// <summary>
/// Skips authorization checks for introspection queries, namely all queries
/// that contain only __schema, __type and __typename top-level fields.
/// </summary>
public class IntrospectionSkipCondition : IAuthorizationSkipCondition
{
/// <inheritdoc />
public ValueTask<bool> ShouldSkip(ValidationContext context)
{
static bool IsIntrospectionField(Field f) => f.Name == "__schema" || f.Name == "__type" || f.Name == "__typename";

bool ContainsOnlyIntrospectionFields(IHaveSelectionSet node)
{
if (node.SelectionSet?.Selections?.Count == 0)
return false; // invalid document, better to return false

foreach (var selection in node.SelectionSet!.Selections)
{
switch (selection)
{
case Field field:
if (!IsIntrospectionField(field))
return false;
break;

case InlineFragment inlineFragment:
if (!ContainsOnlyIntrospectionFields(inlineFragment))
return false;
break;

case FragmentSpread fragmentSpread:
var fragmentDef = context.Document.Fragments.FindDefinition(fragmentSpread.Name);
if (fragmentDef == null || !ContainsOnlyIntrospectionFields(fragmentDef))
return false;
break;

default:
return false;
}
}

return true;
}

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

return new ValueTask<bool>(actualOperation?.OperationType == OperationType.Query
? ContainsOnlyIntrospectionFields(actualOperation)
: false); // not an executable document
}
}
}