diff --git a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt index b0770b7..d4d4d07 100644 --- a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt +++ b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt @@ -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 skipConditions) { } public System.Threading.Tasks.ValueTask ValidateAsync(GraphQL.Validation.ValidationContext context) { } } public class ClaimAuthorizationRequirement : GraphQL.Authorization.IAuthorizationRequirement @@ -80,8 +81,17 @@ namespace GraphQL.Authorization { System.Threading.Tasks.Task Authorize(GraphQL.Authorization.AuthorizationContext context); } + public interface IAuthorizationSkipCondition + { + System.Threading.Tasks.ValueTask 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 ShouldSkip(GraphQL.Validation.ValidationContext context) { } + } } \ No newline at end of file diff --git a/src/GraphQL.Authorization.Tests/AuthorizationSkipTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationSkipTests.cs new file mode 100644 index 0000000..37ef955 --- /dev/null +++ b/src/GraphQL.Authorization.Tests/AuthorizationSkipTests.cs @@ -0,0 +1,87 @@ +using GraphQL.Types; +using Xunit; + +namespace GraphQL.Authorization.Tests +{ + /// + /// Tests for . + /// https://github.com/graphql-dotnet/authorization/issues/28 + /// + 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()); + + [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 +}"; + } +} diff --git a/src/GraphQL.Authorization.Tests/ValidationTestBase.cs b/src/GraphQL.Authorization.Tests/ValidationTestBase.cs index ef7e93d..606a3ff 100644 --- a/src/GraphQL.Authorization.Tests/ValidationTestBase.cs +++ b/src/GraphQL.Authorization.Tests/ValidationTestBase.cs @@ -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; } diff --git a/src/GraphQL.Authorization/AuthorizationValidationRule.cs b/src/GraphQL.Authorization/AuthorizationValidationRule.cs index 99698df..7a1b57e 100644 --- a/src/GraphQL.Authorization/AuthorizationValidationRule.cs +++ b/src/GraphQL.Authorization/AuthorizationValidationRule.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; @@ -14,14 +15,25 @@ namespace GraphQL.Authorization public class AuthorizationValidationRule : IValidationRule { private readonly IAuthorizationEvaluator _evaluator; + private readonly IAuthorizationSkipCondition[] _skipConditions; /// /// Creates an instance of with /// the specified authorization evaluator. /// public AuthorizationValidationRule(IAuthorizationEvaluator evaluator) + : this(evaluator, null!) + { + } + + /// + /// Creates an instance of with + /// the specified authorization evaluator and authorization skip conditions. + /// + public AuthorizationValidationRule(IAuthorizationEvaluator evaluator, IEnumerable skipConditions) { _evaluator = evaluator; + _skipConditions = skipConditions?.ToArray() ?? Array.Empty(); } private bool ShouldBeSkipped(Operation actualOperation, ValidationContext context) @@ -77,8 +89,25 @@ void Visit(INode node, int _) } /// - public ValueTask ValidateAsync(ValidationContext context) + public async ValueTask ValidateAsync(ValidationContext context) { + async ValueTask 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(); @@ -88,7 +117,7 @@ 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(new NodeVisitors( + return new NodeVisitors( new MatchingNodeVisitor((astType, context) => { if (context.Document.Operations.Count > 1 && astType.Name != context.OperationName) @@ -96,6 +125,7 @@ void Visit(INode node, int _) return; } + // Actually, astType always equals actualOperation operationType = astType.OperationType; var type = context.TypeInfo.GetLastType(); @@ -148,7 +178,7 @@ void Visit(INode node, int _) } } }) - )); + ); } private void CheckAuth( diff --git a/src/GraphQL.Authorization/IAuthorizationSkipCondition.cs b/src/GraphQL.Authorization/IAuthorizationSkipCondition.cs new file mode 100644 index 0000000..c4a9acc --- /dev/null +++ b/src/GraphQL.Authorization/IAuthorizationSkipCondition.cs @@ -0,0 +1,17 @@ +using System.Threading.Tasks; +using GraphQL.Validation; + +namespace GraphQL.Authorization +{ + /// + /// Allows to conditionally skip entire AST traversing and all + /// authorization checks in . + /// + public interface IAuthorizationSkipCondition + { + /// + /// Specifies whether authorization checks should be skipped. + /// + ValueTask ShouldSkip(ValidationContext context); + } +} diff --git a/src/GraphQL.Authorization/IntrospectionSkipCondition.cs b/src/GraphQL.Authorization/IntrospectionSkipCondition.cs new file mode 100644 index 0000000..1bfc20c --- /dev/null +++ b/src/GraphQL.Authorization/IntrospectionSkipCondition.cs @@ -0,0 +1,59 @@ +using System.Linq; +using System.Threading.Tasks; +using GraphQL.Language.AST; +using GraphQL.Validation; + +namespace GraphQL.Authorization +{ + /// + /// Skips authorization checks for introspection queries, namely all queries + /// that contain only __schema, __type and __typename top-level fields. + /// + public class IntrospectionSkipCondition : IAuthorizationSkipCondition + { + /// + public ValueTask 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(actualOperation?.OperationType == OperationType.Query + ? ContainsOnlyIntrospectionFields(actualOperation) + : false); // not an executable document + } + } +}