Skip to content
Merged
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
1 change: 1 addition & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ updates:
directory: "/"
schedule:
interval: "daily"

- package-ecosystem: "github-actions"
directory: "/"
schedule:
Expand Down
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=5.0.0-preview-411
run: dotnet restore -p:GraphQLTestVersion=5.1.1

- name: Build solution
working-directory: src
Expand Down
7 changes: 5 additions & 2 deletions .github/workflows/label.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@
# https://github.com/actions/labeler/blob/master/README.md

name: Labeler
on: [pull_request]
on:
pull_request_target:
types:
- opened # when PR is opened

jobs:
label:
runs-on: ubuntu-latest
steps:
- uses: actions/labeler@v3
- uses: actions/labeler@v4
with:
sync-labels: ""
repo-token: "${{ secrets.GITHUB_TOKEN }}"
Expand Down
7 changes: 2 additions & 5 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
- ubuntu-latest
- windows-latest
graphqlversion:
- 5.0.0-preview-411
- 5.1.1
steps:
- name: Checkout source
uses: actions/checkout@v2
Expand All @@ -54,10 +54,7 @@ jobs:
if: ${{ startsWith(matrix.os, 'ubuntu') }}
working-directory: src
run: |
dotnet tool install -g dotnet-format --version 6.0.243104 --add-source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nuget/v3/index.json
dotnet format --no-restore --verify-no-changes -v diag --severity warn whitespace || (echo "Run 'dotnet format' to fix whitespace issues" && exit 1)
dotnet format --no-restore --verify-no-changes -v diag --severity warn analyzers || (echo "Run 'dotnet format' to fix analyzers issues" && exit 1)
dotnet format --no-restore --verify-no-changes -v diag --severity warn style || (echo "Run 'dotnet format' to fix style issues" && exit 1)
dotnet format --no-restore --verify-no-changes --severity warn || (echo "Run 'dotnet format' to fix issues" && exit 1)
- name: Build solution [Release]
working-directory: src
run: dotnet build --no-restore -c Release -p:GraphQLTestVersion=${{ matrix.graphqlversion }}
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="5.0.0-preview-411" />
<PackageReference Include="GraphQL.SystemTextJson" Version="5.1.1" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="6.0.0" />
</ItemGroup>

Expand Down
2 changes: 1 addition & 1 deletion src/BasicSample/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public class Query
/// <summary>
/// Resolver for 'Query.viewer' field.
/// </summary>
[GraphQLAuthorize("AdminPolicy")]
[Authorize("AdminPolicy")]
public User Viewer() => new() { Id = Guid.NewGuid().ToString(), Name = "Quinn" };

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="PublicApiGenerator" Version="10.2.0" />
<PackageReference Include="PublicApiGenerator" Version="10.3.0" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ type Query {
}

[GraphQLMetadata("Query")]
[GraphQLAuthorize("ClassPolicy")]
[Authorize("ClassPolicy")]
public class QueryWithAttributes
{
[GraphQLAuthorize("FieldPolicy")]
[Authorize("FieldPolicy")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0060:Remove unused parameter", Justification = "test")]
public string Post(string id) => "";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,10 +331,10 @@ type Query {
}

[GraphQLMetadata("Query")]
[GraphQLAuthorize("ClassPolicy")]
[Authorize("ClassPolicy")]
public class BasicQueryWithAttributes
{
[GraphQLAuthorize("FieldPolicy")]
[Authorize("FieldPolicy")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0060:Remove unused parameter", Justification = "test")]
public string Post(string id) => "";
}
Expand Down Expand Up @@ -374,7 +374,7 @@ public class NestedQueryWithAttributes
public string? Comment() => null;
}

[GraphQLAuthorize("PostPolicy")]
[Authorize("PostPolicy")]
public class Post
{
public string? Id { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

<PropertyGroup>
<TargetFrameworks>net6;net5;netcoreapp3.1</TargetFrameworks>
<GraphQLTestVersion>5.0.0-preview-411</GraphQLTestVersion>
<GraphQLTestVersion>5.1.1</GraphQLTestVersion>
</PropertyGroup>

<ItemGroup>
Expand Down
11 changes: 10 additions & 1 deletion src/GraphQL.Authorization.Tests/ValidationTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Security.Claims;
using GraphQL.Execution;
using GraphQL.Validation;
using GraphQLParser;
using Shouldly;

namespace GraphQL.Authorization.Tests
Expand Down Expand Up @@ -63,7 +64,15 @@ 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.Variables, config.OperationName).GetAwaiter().GetResult().validationResult;
return validator.ValidateAsync(new ValidationOptions
{
Schema = config.Schema,
Document = document,
Operation = document.OperationWithName(config.OperationName),
Rules = config.Rules,
Variables = config.Variables ?? Inputs.Empty,
UserContext = userContext
}).GetAwaiter().GetResult().validationResult;
}

internal static ClaimsPrincipal CreatePrincipal(string? authenticationType = null, IDictionary<string, string>? claims = null)
Expand Down
111 changes: 60 additions & 51 deletions src/GraphQL.Authorization/AuthorizationValidationRule.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using GraphQL.Language.AST;
using GraphQL.Types;
using GraphQL.Validation;
using GraphQLParser;
using GraphQLParser.AST;
using GraphQLParser.Visitors;

namespace GraphQL.Authorization
{
Expand All @@ -24,9 +27,9 @@ public AuthorizationValidationRule(IAuthorizationEvaluator evaluator)
_evaluator = evaluator;
}

private bool ShouldBeSkipped(Operation actualOperation, ValidationContext context)
private bool ShouldBeSkipped(GraphQLOperationDefinition actualOperation, ValidationContext context)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes are copy-pasted from the server project.

{
if (context.Document.Operations.Count <= 1)
if (context.Document.OperationsCount() <= 1)
{
return false;
}
Expand All @@ -46,137 +49,143 @@ private bool ShouldBeSkipped(Operation actualOperation, ValidationContext contex
return true;
}

if (ancestor is FragmentDefinition fragment)
if (ancestor is GraphQLFragmentDefinition fragment)
{
return !FragmentBelongsToOperation(fragment, actualOperation);
//TODO: may be rewritten completely later
var c = new FragmentBelongsToOperationVisitorContext(fragment);
_visitor.VisitAsync(actualOperation, c).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this
return !c.Found;
}
} while (true);
}

private bool FragmentBelongsToOperation(FragmentDefinition fragment, Operation operation)
private sealed class FragmentBelongsToOperationVisitorContext : IASTVisitorContext
{
bool belongs = false;
void Visit(INode node, int _)
public FragmentBelongsToOperationVisitorContext(GraphQLFragmentDefinition fragment)
{
if (belongs)
{
return;
}
Fragment = fragment;
}

belongs = node is FragmentSpread fragmentSpread && fragmentSpread.Name == fragment.Name;
public GraphQLFragmentDefinition Fragment { get; }

if (node != null)
{
node.Visit(Visit, 0);
}
}
public bool Found { get; set; }

public CancellationToken CancellationToken => default;
}

operation.Visit(Visit, 0);
private static readonly FragmentBelongsToOperationVisitor _visitor = new();

return belongs;
private sealed class FragmentBelongsToOperationVisitor : ASTVisitor<FragmentBelongsToOperationVisitorContext>
{
protected override ValueTask VisitFragmentSpreadAsync(GraphQLFragmentSpread fragmentSpread, FragmentBelongsToOperationVisitorContext context)
{
context.Found = context.Fragment.FragmentName.Name == fragmentSpread.FragmentName.Name;
return default;
}

public override ValueTask VisitAsync(ASTNode? node, FragmentBelongsToOperationVisitorContext context)
{
return context.Found ? default : base.VisitAsync(node, context);
}
}

/// <inheritdoc />
public ValueTask<INodeVisitor?> ValidateAsync(ValidationContext context)
public async ValueTask<INodeVisitor?> ValidateAsync(ValidationContext context)
{
var userContext = context.UserContext as IProvideClaimsPrincipal;
await AuthorizeAsync(null, context.Schema, userContext, context, null);
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 new ValueTask<INodeVisitor?>(new NodeVisitors(
new MatchingNodeVisitor<Operation>((astType, context) =>
return new NodeVisitors(
new MatchingNodeVisitor<GraphQLOperationDefinition>((astType, context) =>
{
if (context.Document.Operations.Count > 1 && astType.Name != context.OperationName)
if (context.Document.OperationsCount() > 1 && astType.Name != context.Operation.Name)
{
return;
}

operationType = astType.OperationType;
operationType = astType.Operation;

var type = context.TypeInfo.GetLastType();
CheckAuth(astType, type, userContext, context, operationType);
AuthorizeAsync(astType, type, userContext, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this
}),

new MatchingNodeVisitor<ObjectField>((objectFieldAst, context) =>
new MatchingNodeVisitor<GraphQLObjectField>((objectFieldAst, context) =>
{
if (context.TypeInfo.GetArgument()?.ResolvedType?.GetNamedType() is IComplexGraphType argumentType && !ShouldBeSkipped(actualOperation, context))
if (context.TypeInfo.GetArgument()?.ResolvedType?.GetNamedType() is IComplexGraphType argumentType && !ShouldBeSkipped(context.Operation, context))
{
var fieldType = argumentType.GetField(objectFieldAst.Name);
CheckAuth(objectFieldAst, fieldType, userContext, context, operationType);
AuthorizeAsync(objectFieldAst, fieldType, userContext, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this
}
}),

new MatchingNodeVisitor<Field>((fieldAst, context) =>
new MatchingNodeVisitor<GraphQLField>((fieldAst, context) =>
{
var fieldDef = context.TypeInfo.GetFieldDef();

if (fieldDef == null || ShouldBeSkipped(actualOperation, context))
if (fieldDef == null || ShouldBeSkipped(context.Operation, context))
return;

// check target field
CheckAuth(fieldAst, fieldDef, userContext, context, operationType);
AuthorizeAsync(fieldAst, fieldDef, userContext, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this
// check returned graph type
CheckAuth(fieldAst, fieldDef.ResolvedType?.GetNamedType(), userContext, context, operationType);
AuthorizeAsync(fieldAst, fieldDef.ResolvedType?.GetNamedType(), userContext, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this
}),

new MatchingNodeVisitor<VariableReference>((variableRef, context) =>
new MatchingNodeVisitor<GraphQLVariable>((variableRef, context) =>
{
if (context.TypeInfo.GetArgument()?.ResolvedType?.GetNamedType() is not IComplexGraphType variableType || ShouldBeSkipped(actualOperation, context))
if (context.TypeInfo.GetArgument()?.ResolvedType?.GetNamedType() is not IComplexGraphType variableType || ShouldBeSkipped(context.Operation, context))
return;

CheckAuth(variableRef, variableType, userContext, context, operationType);
AuthorizeAsync(variableRef, variableType, userContext, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this

// Check each supplied field in the variable that exists in the variable type.
// If some supplied field does not exist in the variable type then some other
// validation rule should check that but here we should just ignore that
// "unknown" field.
if (context.Variables != null &&
context.Variables.TryGetValue(variableRef.Name, out object? input) &&
context.Variables.TryGetValue(variableRef.Name.StringValue, out object? input) && //ISSUE:allocation
input is Dictionary<string, object> fieldsValues)
{
foreach (var field in variableType.Fields)
{
if (fieldsValues.ContainsKey(field.Name))
{
CheckAuth(variableRef, field, userContext, context, operationType);
AuthorizeAsync(variableRef, field, userContext, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this
}
}
}
})
));
);
}

private void CheckAuth(
INode node,
private async Task AuthorizeAsync(
ASTNode? node,
IProvideMetadata? provider,
IProvideClaimsPrincipal? userContext,
ValidationContext context,
OperationType? operationType)
{
if (provider == null || !provider.RequiresAuthorization())
if (provider == null || !provider.IsAuthorizationRequired())
return;

// TODO: async -> sync transition
var result = _evaluator
.Evaluate(userContext?.User, context.UserContext, context.Variables, provider.GetPolicies())
.GetAwaiter()
.GetResult();
var result = await _evaluator.Evaluate(userContext?.User, context.UserContext, context.Variables, provider.GetPolicies());

if (result.Succeeded)
return;

string errors = string.Join("\n", result.Errors);

context.ReportError(new ValidationError(
context.Document.OriginalQuery!,
context.Document.Source,
"authorization",
$"You are not authorized to run this {operationType.ToString().ToLower()}.\n{errors}",
node));
node == null ? Array.Empty<ASTNode>() : new ASTNode[] { node }));
}
}
}
2 changes: 1 addition & 1 deletion src/GraphQL.Authorization/GraphQL.Authorization.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="GraphQL" Version="5.0.0-preview-411" />
<PackageReference Include="GraphQL" Version="5.1.1" />
</ItemGroup>

</Project>
2 changes: 1 addition & 1 deletion src/Harness/GraphQL.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class Query
/// <summary>
/// Resolver for 'Query.viewer' field.
/// </summary>
[GraphQLAuthorize("AdminPolicy")]
[Authorize("AdminPolicy")]
public User Viewer() => new() { Id = Guid.NewGuid().ToString(), Name = "Quinn" };

/// <summary>
Expand Down
Loading