Skip to content

Commit

Permalink
Small cleanup and fix #61 (#87)
Browse files Browse the repository at this point in the history
  • Loading branch information
sungam3r authored Nov 3, 2020
1 parent a590608 commit 4821115
Show file tree
Hide file tree
Showing 24 changed files with 281 additions and 135 deletions.
1 change: 1 addition & 0 deletions .github/labeler.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"test":
- src/GraphQL.Authorization.Tests/**/*
- src/GraphQL.Authorization.ApiTests/**/*

"CI":
- .github/workflows/**/*
Expand Down
1 change: 1 addition & 0 deletions src/BasicSample/BasicSample.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp3.1</TargetFramework>
<IsPackable>false</IsPackable>
<NoWarn>$(NoWarn);1591</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/Directory.Build.props
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project>

<PropertyGroup>
<VersionPrefix>3.0.44-preview</VersionPrefix>
<VersionPrefix>3.1.0-preview</VersionPrefix>
<LangVersion>latest</LangVersion>
<Authors>Joe McBride</Authors>
<PackageLicenseExpression>MIT</PackageLicenseExpression>
Expand Down
4 changes: 2 additions & 2 deletions src/GraphQL.Authorization.ApiTests/ApiApprovalTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public class ApiApprovalTests
{
[Theory]
[InlineData(typeof(IAuthorizationRequirement))]
public void PublicApi(Type type)
public void public_api_should_not_change_unintentionally(Type type)
{
string publicApi = type.Assembly.GeneratePublicApi(new ApiGeneratorOptions
{
Expand All @@ -21,7 +21,7 @@ public void PublicApi(Type type)
// Note: If the AssemblyName.approved.txt file doesn't match the latest publicApi value,
// this call will try to launch a diff tool to help you out but that can fail on
// your machine if a diff tool isn't configured/setup.
publicApi.ShouldMatchApproved(options => options.WithDiscriminator(type.Assembly.GetName().Name));
publicApi.ShouldMatchApproved(options => options.WithFilenameGenerator((testMethodInfo, discriminator, fileType, fileExtension) => $"{type.Assembly.GetName().Name}.{fileType}.{fileExtension}"));
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netcoreapp3.1</TargetFramework>
<NoWarn>$(NoWarn);1591</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand All @@ -15,5 +16,5 @@
<ItemGroup>
<ProjectReference Include="..\GraphQL.Authorization\GraphQL.Authorization.csproj" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ namespace GraphQL.Authorization
{
public ClaimAuthorizationRequirement(string claimType) { }
public ClaimAuthorizationRequirement(string claimType, System.Collections.Generic.IEnumerable<string> allowedValues) { }
public ClaimAuthorizationRequirement(string claimType, params string[] allowedValues) { }
public ClaimAuthorizationRequirement(string claimType, System.Collections.Generic.IEnumerable<string> allowedValues, System.Collections.Generic.IEnumerable<string> displayValues) { }
public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.AuthorizationContext context) { }
}
Expand Down
35 changes: 27 additions & 8 deletions src/GraphQL.Authorization.Tests/AuthorizationEvaluatorTests.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.Security.Claims;
using System.Threading.Tasks;
Expand Down Expand Up @@ -26,7 +27,7 @@ public async Task fails_with_null_principal()
null,
null,
null,
new[] {"MyPolicy"}
new[] { "MyPolicy" }
);

result.Succeeded.ShouldBeFalse();
Expand All @@ -41,7 +42,7 @@ public async Task fails_when_missing_claim()
CreatePrincipal(),
null,
null,
new[] {"MyPolicy"}
new[] { "MyPolicy" }
);

result.Succeeded.ShouldBeFalse();
Expand All @@ -55,7 +56,7 @@ public async Task fails_when_missing_policy()
var result = await _evaluator.Evaluate(
CreatePrincipal(claims: new Dictionary<string, string>
{
{"Admin", "true"}
{ "Admin", "true" }
}),
null,
null,
Expand All @@ -73,11 +74,11 @@ public async Task succeeds_when_policy_applied()
var result = await _evaluator.Evaluate(
CreatePrincipal(claims: new Dictionary<string, string>
{
{"Admin", "true"}
{ "Admin", "true" }
}),
null,
null,
new[] {"MyPolicy"}
new[] { "MyPolicy" }
);

result.Succeeded.ShouldBeTrue();
Expand All @@ -91,11 +92,11 @@ public async Task succeeds_with_claim_value()
var result = await _evaluator.Evaluate(
CreatePrincipal(claims: new Dictionary<string, string>
{
{"Admin", "true"}
{ "Admin", "true" }
}),
null,
null,
new[] {"MyPolicy"}
new[] { "MyPolicy" }
);

result.Succeeded.ShouldBeTrue();
Expand All @@ -109,7 +110,7 @@ public async Task succeeds_when_null_policies()
var result = await _evaluator.Evaluate(
CreatePrincipal(claims: new Dictionary<string, string>
{
{"Admin", "true"}
{ "Admin", "true" }
}),
null,
null,
Expand All @@ -119,6 +120,24 @@ public async Task succeeds_when_null_policies()
result.Succeeded.ShouldBeTrue();
}

[Fact]
public async Task succeeds_when_empty_policies()
{
_settings.AddPolicy("MyPolicy", _ => { });

var result = await _evaluator.Evaluate(
CreatePrincipal(claims: new Dictionary<string, string>
{
{ "Admin", "true" }
}),
null,
null,
Array.Empty<string>()
);

result.Succeeded.ShouldBeTrue();
}

[Fact]
public async Task succeeds_when_null_principal()
{
Expand Down
44 changes: 29 additions & 15 deletions src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ public class AuthorizationValidationRuleTests : ValidationTestBase
[Fact]
public void class_policy_success()
{
Settings.AddPolicy("ClassPolicy", _ => _.RequireClaim("admin"));
Settings.AddPolicy("FieldPolicy", _ => _.RequireClaim("admin"));
Settings.AddPolicy("ClassPolicy", builder => builder.RequireClaim("admin"));
Settings.AddPolicy("FieldPolicy", builder => builder.RequireClaim("admin"));

ShouldPassRule(_ =>
{
_.Query = @"query { post }";
_.Schema = BasicSchema();
_.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{"Admin", "true"}
{ "Admin", "true" }
});
});
}
Expand All @@ -39,16 +39,16 @@ public void class_policy_fail()
[Fact]
public void field_policy_success()
{
Settings.AddPolicy("ClassPolicy", _ => _.RequireClaim("admin"));
Settings.AddPolicy("FieldPolicy", _ => _.RequireClaim("admin"));
Settings.AddPolicy("ClassPolicy", builder => builder.RequireClaim("admin"));
Settings.AddPolicy("FieldPolicy", builder => builder.RequireClaim("admin"));

ShouldPassRule(_ =>
{
_.Query = @"query { post }";
_.Schema = BasicSchema();
_.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{"Admin", "true"}
{ "Admin", "true" }
});
});
}
Expand Down Expand Up @@ -76,7 +76,7 @@ public void nested_type_policy_success()
_.Schema = NestedSchema();
_.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{"Admin", "true"}
{ "Admin", "true" }
});
});
}
Expand Down Expand Up @@ -128,7 +128,7 @@ public void passes_with_claim_on_input_type()
_.Schema = TypedSchema();
_.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{"Admin", "true"}
{ "Admin", "true" }
});
});
}
Expand All @@ -148,17 +148,31 @@ public void fails_on_missing_claim_on_input_type()
[Fact]
public void passes_with_multiple_policies_on_field_and_single_on_input_type()
{
Settings.AddPolicy("FieldPolicy", _ => _.RequireClaim("admin"));
Settings.AddPolicy("AdminPolicy", _ => _.RequireClaim("admin"));
Settings.AddPolicy("ConfidentialPolicy", _ => _.RequireClaim("admin"));
Settings.AddPolicy("FieldPolicy", builder => builder.RequireClaim("admin"));
Settings.AddPolicy("AdminPolicy", builder => builder.RequireClaim("admin"));
Settings.AddPolicy("ConfidentialPolicy", builder => builder.RequireClaim("admin"));

ShouldPassRule(_ =>
{
_.Query = @"query { author(input: { name: ""Quinn"" }) project(input: { name: ""TEST"" }) }";
_.Schema = TypedSchema();
_.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{"Admin", "true"}
{ "Admin", "true" }
});
});
}

[Fact]
public void Issue61()
{
ShouldPassRule(_ =>
{
_.Query = @"query { unknown(obj: {id: 7}) }";
_.Schema = TypedSchema();
_.User = CreatePrincipal(claims: new Dictionary<string, string>
{
{ "Admin", "true" }
});
});
}
Expand Down Expand Up @@ -228,10 +242,10 @@ type Post {
}
";

return Schema.For(defs, _ =>
return Schema.For(defs, builder =>
{
_.Types.Include<NestedQueryWithAttributes>();
_.Types.Include<Post>();
builder.Types.Include<NestedQueryWithAttributes>();
builder.Types.Include<Post>();
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public async Task produces_error_when_missing_claim_ignoring_value()
[Fact]
public async Task produces_error_when_missing_claim_with_single_value()
{
var req = new ClaimAuthorizationRequirement("Admin", new[] {"true"});
var req = new ClaimAuthorizationRequirement("Admin", "true");

var context = new AuthorizationContext
{
Expand All @@ -44,7 +44,7 @@ public async Task produces_error_when_missing_claim_with_single_value()
[Fact]
public async Task produces_error_when_missing_claim_with_multiple_values()
{
var req = new ClaimAuthorizationRequirement("Admin", new[] {"true", "maybe"});
var req = new ClaimAuthorizationRequirement("Admin", "true", "maybe");

var context = new AuthorizationContext
{
Expand Down Expand Up @@ -75,7 +75,7 @@ public async Task succeeds_when_claim_with_ignoring_value()
[Fact]
public async Task succeeds_when_claim_with_single_value()
{
var req = new ClaimAuthorizationRequirement("Admin", new[] {"true"});
var req = new ClaimAuthorizationRequirement("Admin", "true");

var context = new AuthorizationContext
{
Expand All @@ -90,7 +90,7 @@ public async Task succeeds_when_claim_with_single_value()
[Fact]
public async Task succeeds_when_claim_with_multiple_values()
{
var req = new ClaimAuthorizationRequirement("Admin", new[] {"true", "maybe"});
var req = new ClaimAuthorizationRequirement("Admin", "true", "maybe");

var context = new AuthorizationContext
{
Expand Down
10 changes: 10 additions & 0 deletions src/GraphQL.Authorization.Tests/GraphQLUserContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using System.Collections.Generic;
using System.Security.Claims;

namespace GraphQL.Authorization.Tests
{
internal class GraphQLUserContext : Dictionary<string, object>, IProvideClaimsPrincipal
{
public ClaimsPrincipal User { get; set; }
}
}
7 changes: 1 addition & 6 deletions src/GraphQL.Authorization.Tests/ValidationTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ public void Rule(params IValidationRule[] rules)
}
}

public class GraphQLUserContext : Dictionary<string, object>, IProvideClaimsPrincipal
{
public ClaimsPrincipal User { get; set; }
}

public class ValidationTestBase
{
public ValidationTestBase()
Expand Down Expand Up @@ -82,7 +77,7 @@ private IValidationResult Validate(ValidationTestConfig config)
var documentBuilder = new GraphQLDocumentBuilder();
var document = documentBuilder.Build(config.Query);
var validator = new DocumentValidator();
return validator.ValidateAsync(config.Query, config.Schema, document, config.Rules, userContext, config.Inputs).GetAwaiter().GetResult(); ;
return validator.ValidateAsync(config.Query, config.Schema, document, config.Rules, userContext, config.Inputs).GetAwaiter().GetResult();
}

protected ClaimsPrincipal CreatePrincipal(string authenticationType = null, IDictionary<string, string> claims = null)
Expand Down
31 changes: 21 additions & 10 deletions src/GraphQL.Authorization/AuthorizationPolicy.cs
Original file line number Diff line number Diff line change
@@ -1,22 +1,33 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;

namespace GraphQL.Authorization
{
public interface IAuthorizationPolicy
{
IEnumerable<IAuthorizationRequirement> Requirements { get; }
}

/// <summary>
/// Default implementation for <see cref="IAuthorizationPolicy"/>.
/// </summary>
public class AuthorizationPolicy : IAuthorizationPolicy
{
private readonly List<IAuthorizationRequirement> _requirements;
private readonly List<IAuthorizationRequirement> _requirements = new List<IAuthorizationRequirement>();

/// <summary>
/// Creates a policy with a set of specified requirements.
/// </summary>
/// <param name="requirements">Specified requirements.</param>
public AuthorizationPolicy(IEnumerable<IAuthorizationRequirement> requirements)
{
_requirements = new List<IAuthorizationRequirement>(
requirements ?? new List<IAuthorizationRequirement>());
if (requirements != null)
{
_requirements.AddRange(requirements);
_requirements.ForEach(req =>
{
if (req == null)
throw new ArgumentNullException(nameof(requirements), $"One of the ({_requirements.Count}) requirements is null");
});
}
}

/// <inheritdoc />
public IEnumerable<IAuthorizationRequirement> Requirements => _requirements;
}
}
}
Loading

0 comments on commit 4821115

Please sign in to comment.